Add `InstanceApiClient` and incorporate `getAuthorizationHeaderFor` i… · github/codeql-action@8884b5b · GitHub
Skip to content

Commit 8884b5b

Browse files
mbgmbg
committed
Add InstanceApiClient and incorporate getAuthorizationHeaderFor into it
1 parent b2bf750 commit 8884b5b

7 files changed

Lines changed: 155 additions & 125 deletions

File tree

‎lib/entry-points.js‎

Lines changed: 35 additions & 19 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/api-client.test.ts‎

Lines changed: 31 additions & 56 deletions

‎src/api-client.ts‎

Lines changed: 55 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,61 @@ export function makeProxyRequestOptions(
122122
/** The type of GitHub API client we use. */
123123
export type ApiClient = InstanceType<typeof githubUtils.GitHub>;
124124

125+
/**
126+
* Encapsulates a {@link GitHubApiCombinedDetails} along with operations which ensure that any credentials
127+
* are only used with the GitHub instance that is represented by the {@link GitHubApiCombinedDetails} value.
128+
*/
129+
export class InstanceApiClient {
130+
private readonly client: ApiClient;
131+
private readonly apiDetails: GitHubApiCombinedDetails;
132+
private readonly authorizationHeader: string | undefined;
133+
134+
constructor(
135+
apiDetails: GitHubApiCombinedDetails,
136+
clientOptions: CreateApiClientOptions = {},
137+
) {
138+
this.apiDetails = apiDetails;
139+
this.client = createApiClientWithDetails(apiDetails, clientOptions);
140+
141+
// Compute the Authorization header value if we have a token.
142+
const token = this.apiDetails.auth?.trim();
143+
if (token !== undefined && token !== "") {
144+
this.authorizationHeader = `token ${token}`;
145+
}
146+
}
147+
148+
/**
149+
* Gets a value for the `Authorization` header for a request to `url`; or `undefined` if the
150+
* `Authorization` header should not be set for `url`.
151+
*
152+
* @param logger The logger to use for debugging messages.
153+
* @param url The URL for which we want to add an `Authorization` header.
154+
*/
155+
public getAuthorizationHeaderFor(
156+
logger: Logger,
157+
url: string,
158+
): string | undefined {
159+
// We don't have to check the `url` if we don't have a token to begin with.
160+
if (this.authorizationHeader === undefined) {
161+
return undefined;
162+
}
163+
164+
// We only want to provide an authorization header if we are downloading
165+
// from the same GitHub instance the Action is running on.
166+
// This avoids leaking Enterprise tokens to dotcom.
167+
if (
168+
url.startsWith(`${this.apiDetails.url}/`) ||
169+
(this.apiDetails.apiURL && url.startsWith(`${this.apiDetails.apiURL}/`))
170+
) {
171+
logger.debug(`Providing an authorization token.`);
172+
return this.authorizationHeader;
173+
}
174+
175+
logger.debug(`Not using an authorization token.`);
176+
return undefined;
177+
}
178+
}
179+
125180
/** Options for `createApiClientWithDetails`. */
126181
interface CreateApiClientOptions {
127182
allowExternal?: boolean;
@@ -218,42 +273,6 @@ export function getApiClientWithExternalAuth(
218273
return createApiClientWithDetails(apiDetails, { allowExternal: true, proxy });
219274
}
220275

221-
/**
222-
* Gets a value for the `Authorization` header for a request to `url`; or `undefined` if the
223-
* `Authorization` header should not be set for `url`.
224-
*
225-
* @param logger The logger to use for debugging messages.
226-
* @param apiDetails Details of the GitHub API we are using.
227-
* @param url The URL for which we want to add an `Authorization` header.
228-
*
229-
* @returns The value for the `Authorization` header or `undefined` if it shouldn't be populated.
230-
*/
231-
export function getAuthorizationHeaderFor(
232-
logger: Logger,
233-
apiDetails: GitHubApiDetails,
234-
url: string,
235-
): string | undefined {
236-
// We don't have to check the `url` if we don't have a token to begin with.
237-
const token = apiDetails.auth?.trim();
238-
if (token === undefined || token === "") {
239-
return undefined;
240-
}
241-
242-
// We only want to provide an authorization header if we are downloading
243-
// from the same GitHub instance the Action is running on.
244-
// This avoids leaking Enterprise tokens to dotcom.
245-
if (
246-
url.startsWith(`${apiDetails.url}/`) ||
247-
(apiDetails.apiURL && url.startsWith(`${apiDetails.apiURL}/`))
248-
) {
249-
logger.debug(`Providing an authorization token.`);
250-
return `token ${token}`;
251-
}
252-
253-
logger.debug(`Not using an authorization token.`);
254-
return undefined;
255-
}
256-
257276
let cachedGitHubVersion: GitHubVersion | undefined = undefined;
258277

259278
export async function getGitHubVersionFromApi(

‎src/setup-codeql.test.ts‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1290,7 +1290,14 @@ for (const fallback of [false, true]) {
12901290
request: { fetch: fetchRelease },
12911291
});
12921292
sinon.stub(api, "getApiClient").value(() => client);
1293-
const authorizationSpy = sinon.spy(api, "getAuthorizationHeaderFor");
1293+
1294+
const spyClient = new api.InstanceApiClient(SAMPLE_DOTCOM_API_DETAILS);
1295+
const authorizationSpy = sinon.spy(
1296+
spyClient,
1297+
"getAuthorizationHeaderFor",
1298+
);
1299+
sinon.stub(api, "InstanceApiClient").returns(spyClient);
1300+
12941301
const extractStub = stubDownloadAndExtract();
12951302
if (fallback) {
12961303
extractStub.onFirstCall().rejects(new HTTPError("Not Found", 404));
@@ -1323,9 +1330,9 @@ for (const fallback of [false, true]) {
13231330
t.is(extractStub.firstCall.args[0], assetURL);
13241331
t.is(extractStub.lastCall.args[0], fallback ? combinedURL : assetURL);
13251332
t.is(authorizationSpy.callCount, extractStub.callCount);
1326-
t.is(authorizationSpy.firstCall.args[2], assetURL);
1333+
t.is(authorizationSpy.firstCall.args[1], assetURL);
13271334
t.is(
1328-
authorizationSpy.lastCall.args[2],
1335+
authorizationSpy.lastCall.args[1],
13291336
fallback ? combinedURL : assetURL,
13301337
);
13311338
t.is(extractStub.lastCall.args[3], "token token");

‎src/setup-codeql.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -767,11 +767,8 @@ export const downloadCodeQL = async function (
767767
if (searchParams.has("token")) {
768768
logger.debug("CodeQL tools URL contains an authorization token.");
769769
} else {
770-
authorization = api.getAuthorizationHeaderFor(
771-
logger,
772-
source.apiDetails,
773-
codeqlURL,
774-
);
770+
const client = new api.InstanceApiClient(source.apiDetails);
771+
authorization = client.getAuthorizationHeaderFor(logger, codeqlURL);
775772
}
776773

777774
const toolcacheDestination = getToolcacheDestination({ logger }, source);

‎src/start-proxy.test.ts‎

Lines changed: 18 additions & 2 deletions

0 commit comments

Comments
 (0)