Add InstanceApiClient and incorporate getAuthorizationHeaderFor into it

Author: mbg

lib/entry-points.js

@@ -309,70 +309,45 @@ test.serial(
 test("getAuthorizationHeaderFor - returns undefined if there's no token", (t) => {
   const logger = new RecordingLogger();
 
-  t.is(
-    api.getAuthorizationHeaderFor(
-      logger,
-      { ...SAMPLE_DOTCOM_API_DETAILS, auth: undefined },
-      `${GITHUB_DOTCOM_URL}/github`,
-    ),
-    undefined,
-  );
-  t.is(
-    api.getAuthorizationHeaderFor(
-      logger,
-      { ...SAMPLE_DOTCOM_API_DETAILS, auth: "" },
-      `${GITHUB_DOTCOM_URL}/github`,
-    ),
-    undefined,
-  );
-  t.is(
-    api.getAuthorizationHeaderFor(
-      logger,
-      { ...SAMPLE_DOTCOM_API_DETAILS, auth: "    " },
-      `${GITHUB_DOTCOM_URL}/github`,
-    ),
-    undefined,
-  );
+  for (const authValue of [undefined, "", "    "]) {
+    const client = new api.InstanceApiClient({
+      ...SAMPLE_DOTCOM_API_DETAILS,
+      auth: authValue,
+    });
+    t.is(
+      client.getAuthorizationHeaderFor(logger, `${GITHUB_DOTCOM_URL}/github`),
+      undefined,
+    );
+  }
 });
 
 test("getAuthorizationHeaderFor - returns header value with token if url is allowed", (t) => {
   const logger = new RecordingLogger();
+  const client = new api.InstanceApiClient({
+    ...SAMPLE_DOTCOM_API_DETAILS,
+  });
 
-  t.is(
-    api.getAuthorizationHeaderFor(
-      logger,
-      SAMPLE_DOTCOM_API_DETAILS,
-      `${GITHUB_DOTCOM_URL}/github`,
-    ),
-    `token ${SAMPLE_DOTCOM_API_DETAILS.auth}`,
-  );
-  t.is(
-    api.getAuthorizationHeaderFor(
-      logger,
-      SAMPLE_DOTCOM_API_DETAILS,
-      `${GITHUB_DOTCOM_API_URL}/github`,
-    ),
-    `token ${SAMPLE_DOTCOM_API_DETAILS.auth}`,
-  );
+  for (const url of [
+    `${GITHUB_DOTCOM_URL}/github`,
+    `${GITHUB_DOTCOM_API_URL}/github`,
+  ]) {
+    t.is(
+      client.getAuthorizationHeaderFor(logger, url),
+      `token ${SAMPLE_DOTCOM_API_DETAILS.auth}`,
+    );
+  }
 });
 
 test("getAuthorizationHeaderFor - returns undefined if url is not allowed", (t) => {
   const logger = new RecordingLogger();
+  const client = new api.InstanceApiClient({
+    ...SAMPLE_DOTCOM_API_DETAILS,
+  });
 
-  t.is(
-    api.getAuthorizationHeaderFor(
-      logger,
-      SAMPLE_DOTCOM_API_DETAILS,
-      `https://localhost/github`,
-    ),
-    undefined,
-  );
-  t.is(
-    api.getAuthorizationHeaderFor(
-      logger,
-      SAMPLE_DOTCOM_API_DETAILS,
-      `https://api.localhost/github`,
-    ),
-    undefined,
-  );
+  for (const url of [
+    `https://localhost/github`,
+    `https://api.localhost/github`,
+  ]) {
+    t.is(client.getAuthorizationHeaderFor(logger, url), undefined);
+  }
 });

src/api-client.test.ts

@@ -122,6 +122,61 @@ export function makeProxyRequestOptions(
 /** The type of GitHub API client we use. */
 export type ApiClient = InstanceType<typeof githubUtils.GitHub>;
 
+/**
+ * Encapsulates a {@link GitHubApiCombinedDetails} along with operations which ensure that any credentials
+ * are only used with the GitHub instance that is represented by the {@link GitHubApiCombinedDetails} value.
+ */
+export class InstanceApiClient {
+  private readonly client: ApiClient;
+  private readonly apiDetails: GitHubApiCombinedDetails;
+  private readonly authorizationHeader: string | undefined;
+
+  constructor(
+    apiDetails: GitHubApiCombinedDetails,
+    clientOptions: CreateApiClientOptions = {},
+  ) {
+    this.apiDetails = apiDetails;
+    this.client = createApiClientWithDetails(apiDetails, clientOptions);
+
+    // Compute the Authorization header value if we have a token.
+    const token = this.apiDetails.auth?.trim();
+    if (token !== undefined && token !== "") {
+      this.authorizationHeader = `token ${token}`;
+    }
+  }
+
+  /**
+   * Gets a value for the `Authorization` header for a request to `url`; or `undefined` if the
+   * `Authorization` header should not be set for `url`.
+   *
+   * @param logger The logger to use for debugging messages.
+   * @param url The URL for which we want to add an `Authorization` header.
+   */
+  public getAuthorizationHeaderFor(
+    logger: Logger,
+    url: string,
+  ): string | undefined {
+    // We don't have to check the `url` if we don't have a token to begin with.
+    if (this.authorizationHeader === undefined) {
+      return undefined;
+    }
+
+    // We only want to provide an authorization header if we are downloading
+    // from the same GitHub instance the Action is running on.
+    // This avoids leaking Enterprise tokens to dotcom.
+    if (
+      url.startsWith(`${this.apiDetails.url}/`) ||
+      (this.apiDetails.apiURL && url.startsWith(`${this.apiDetails.apiURL}/`))
+    ) {
+      logger.debug(`Providing an authorization token.`);
+      return this.authorizationHeader;
+    }
+
+    logger.debug(`Not using an authorization token.`);
+    return undefined;
+  }
+}
+
 /** Options for `createApiClientWithDetails`. */
 interface CreateApiClientOptions {
   allowExternal?: boolean;
@@ -207,42 +262,6 @@ export function getApiClientWithExternalAuth(
   return createApiClientWithDetails(apiDetails, { allowExternal: true, proxy });
 }
 
-/**
- * Gets a value for the `Authorization` header for a request to `url`; or `undefined` if the
- * `Authorization` header should not be set for `url`.
- *
- * @param logger The logger to use for debugging messages.
- * @param apiDetails Details of the GitHub API we are using.
- * @param url The URL for which we want to add an `Authorization` header.
- *
- * @returns The value for the `Authorization` header or `undefined` if it shouldn't be populated.
- */
-export function getAuthorizationHeaderFor(
-  logger: Logger,
-  apiDetails: GitHubApiDetails,
-  url: string,
-): string | undefined {
-  // We don't have to check the `url` if we don't have a token to begin with.
-  const token = apiDetails.auth?.trim();
-  if (token === undefined || token === "") {
-    return undefined;
-  }
-
-  // We only want to provide an authorization header if we are downloading
-  // from the same GitHub instance the Action is running on.
-  // This avoids leaking Enterprise tokens to dotcom.
-  if (
-    url.startsWith(`${apiDetails.url}/`) ||
-    (apiDetails.apiURL && url.startsWith(`${apiDetails.apiURL}/`))
-  ) {
-    logger.debug(`Providing an authorization token.`);
-    return `token ${token}`;
-  }
-
-  logger.debug(`Not using an authorization token.`);
-  return undefined;
-}
-
 let cachedGitHubVersion: GitHubVersion | undefined = undefined;
 
 export async function getGitHubVersionFromApi(

src/api-client.ts

@@ -1290,7 +1290,14 @@ for (const fallback of [false, true]) {
         request: { fetch: fetchRelease },
       });
       sinon.stub(api, "getApiClient").value(() => client);
-      const authorizationSpy = sinon.spy(api, "getAuthorizationHeaderFor");
+
+      const spyClient = new api.InstanceApiClient(SAMPLE_DOTCOM_API_DETAILS);
+      const authorizationSpy = sinon.spy(
+        spyClient,
+        "getAuthorizationHeaderFor",
+      );
+      sinon.stub(api, "InstanceApiClient").returns(spyClient);
+
       const extractStub = stubDownloadAndExtract();
       if (fallback) {
         extractStub.onFirstCall().rejects(new HTTPError("Not Found", 404));
@@ -1323,9 +1330,9 @@ for (const fallback of [false, true]) {
         t.is(extractStub.firstCall.args[0], assetURL);
         t.is(extractStub.lastCall.args[0], fallback ? combinedURL : assetURL);
         t.is(authorizationSpy.callCount, extractStub.callCount);
-        t.is(authorizationSpy.firstCall.args[2], assetURL);
+        t.is(authorizationSpy.firstCall.args[1], assetURL);
         t.is(
-          authorizationSpy.lastCall.args[2],
+          authorizationSpy.lastCall.args[1],
           fallback ? combinedURL : assetURL,
         );
         t.is(extractStub.lastCall.args[3], "token token");

src/setup-codeql.test.ts

@@ -767,11 +767,8 @@ export const downloadCodeQL = async function (
   if (searchParams.has("token")) {
     logger.debug("CodeQL tools URL contains an authorization token.");
   } else {
-    authorization = api.getAuthorizationHeaderFor(
-      logger,
-      source.apiDetails,
-      codeqlURL,
-    );
+    const client = new api.InstanceApiClient(source.apiDetails);
+    authorization = client.getAuthorizationHeaderFor(logger, codeqlURL);
   }
 
   const toolcacheDestination = getToolcacheDestination({ logger }, source);