Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Offer an option to specify a cache for conditionally fetching public keys #81

Merged
merged 42 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
b844d07
feat: implement cache + conditional request support
francisfuzz Sep 12, 2024
5425c02
fix: run lint:fix and commit changes
francisfuzz Sep 12, 2024
373e252
test: include assertion for cache
francisfuzz Sep 12, 2024
9a48b50
fix: update test name for setup
francisfuzz Sep 12, 2024
54b514b
docs: leave TODO
francisfuzz Sep 12, 2024
4d06267
fix: update signature to return complete cache
francisfuzz Sep 12, 2024
94c111e
refactor: define VerificationKeysCache type
francisfuzz Sep 12, 2024
d421896
refactor: drop export
francisfuzz Sep 12, 2024
73ae790
fix: use key, not keyId
francisfuzz Sep 12, 2024
e62932a
fix: use standardized id value
francisfuzz Sep 12, 2024
029c584
fix: lint
francisfuzz Sep 12, 2024
6fdeb95
refactor: use existing type
francisfuzz Sep 12, 2024
d10e983
fix: use explicit assertions in typescript tests
francisfuzz Sep 12, 2024
8cda7cc
refactor: de-dupe VerifyRequestInterface
francisfuzz Sep 12, 2024
cd20041
fix: remove unused dep
francisfuzz Sep 12, 2024
58f4606
test: return cache when 304
francisfuzz Sep 12, 2024
d62dd94
Update index.d.ts
francisfuzz Sep 13, 2024
4828168
refactor: scope constant for reuse
francisfuzz Sep 13, 2024
81a6f33
feat: return cache key in .verifyAndParseRequest
francisfuzz Sep 13, 2024
7a740cc
Merge branch 'main' into 9-cache
francisfuzz Sep 16, 2024
6322126
docs: update verifyRequestByKeyId example
francisfuzz Sep 16, 2024
693ca29
docs: update fetchVerificationKeys
francisfuzz Sep 16, 2024
5397672
docs: update verifyAndParseRequest
francisfuzz Sep 16, 2024
2cb7b3a
docs: update verifyRequestByKeyId
francisfuzz Sep 16, 2024
73f15e4
fix: drop brackets
francisfuzz Sep 16, 2024
903cdfe
fix: remove newline
francisfuzz Sep 16, 2024
c7b600e
test: assert cache arg is acceptable
francisfuzz Sep 16, 2024
c4320eb
fix: remove extra backticks
francisfuzz Sep 16, 2024
fb4f95c
test: add test for cache verification
francisfuzz Sep 16, 2024
ec40132
fix: add newline back
francisfuzz Sep 16, 2024
8389fc2
fix: remove example script
francisfuzz Sep 16, 2024
f9707c5
fix: add newline between section
francisfuzz Sep 16, 2024
5fa5d20
docs: correct true method name
francisfuzz Sep 16, 2024
15ec544
docs: annotate isValidRequest
francisfuzz Sep 16, 2024
13fbe9e
docs: update constants
francisfuzz Sep 16, 2024
a975a31
test: update mock etag values
francisfuzz Sep 16, 2024
861f2e3
Merge branch 'main' into 9-cache
francisfuzz Sep 17, 2024
2cca2a9
Merge branch 'main' into 9-cache
gr2m Sep 18, 2024
659a108
test: adapt latest changes from `main` to conditional request APIs
gr2m Sep 18, 2024
47c473e
style: prettier
gr2m Sep 18, 2024
e71f3f1
use lowercase header, use same etag value as in other tests
gr2m Sep 18, 2024
d3ebeb5
Update lib/verification.js
gr2m Sep 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ type RequestInterface = typeof request;
type RequestOptions = {
request?: RequestInterface;
token?: string;
cache?: {
id: string;
keys: VerificationPublicKey[];
};
};
export type VerificationPublicKey = {
key_identifier: string;
Expand All @@ -18,7 +22,13 @@ interface VerifyRequestInterface {
}

interface FetchVerificationKeysInterface {
(requestOptions?: RequestOptions): Promise<VerificationPublicKey[]>;
(
requestOptions?: RequestOptions,
): Promise<{ cacheId: string; keys: VerificationPublicKey[] }>;
}

interface VerifyRequest {
(rawBody: string, signature: string, keyId: string): boolean;
}

interface VerifyRequestByKeyIdInterface {
Expand All @@ -27,7 +37,7 @@ interface VerifyRequestByKeyIdInterface {
signature: string,
keyId: string,
requestOptions?: RequestOptions,
): Promise<boolean>;
): Promise<{ isValid: boolean; cacheId: string }>;
}

// response types
Expand Down
7 changes: 5 additions & 2 deletions index.test-d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export async function verifyRequestByKeyIdTest(
keyId: string,
) {
const result = await verifyRequestByKeyId(rawBody, signature, keyId);
expectType<boolean>(result);
expectType<{ isValid: boolean; cacheId: string }>(result);

// @ts-expect-error - first 3 arguments are required
verifyRequestByKeyId(rawBody, signature);
Expand Down Expand Up @@ -76,13 +76,16 @@ export async function verifyRequestTest(

export async function fetchVerificationKeysTest() {
const result = await fetchVerificationKeys();
expectType<VerificationPublicKey[]>(result);
expectType<{ cacheId: string; keys: VerificationPublicKey[] }>(result);

// accepts a token argument
await fetchVerificationKeys({ token });

// accepts a request argument
await fetchVerificationKeys({ request });

// accepts a cache argument
await fetchVerificationKeys({ cache: { id: "test", keys: [] } });
}

export function createAckEventTest() {
Expand Down
4 changes: 2 additions & 2 deletions lib/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,15 @@ export function transformPayloadForOpenAICompatibility(payload) {

/** @type {import('..').VerifyAndParseRequestInterface} */
export async function verifyAndParseRequest(body, signature, keyID, options) {
const isValidRequest = await verifyRequestByKeyId(
const { isValid } = await verifyRequestByKeyId(
body,
signature,
keyID,
options
);

return {
isValidRequest,
isValidRequest: isValid,
payload: parseRequestBody(body),
};
}
Expand Down
38 changes: 24 additions & 14 deletions lib/verification.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,36 +4,45 @@ import { createVerify } from "node:crypto";

import { request as defaultRequest } from "@octokit/request";

/** @type {import('..').VerifyRequestByKeyIdInterface} */
export async function verifyRequest(rawBody, signature, key) {
/** @type {import('..').VerifyRequest} */
export async function verifyRequest(rawBody, signature, keyId) {
// verify arguments
assertValidString(rawBody, "Invalid payload");
assertValidString(signature, "Invalid signature");
assertValidString(key, "Invalid key");
assertValidString(keyId, "Invalid key");

// verify signature
try {
return createVerify("SHA256")
.update(rawBody)
.verify(key, signature, "base64");
.verify(keyId, signature, "base64");
} catch {
return false;
}
}

/** @type {import('..').FetchVerificationKeysInterface} */
export async function fetchVerificationKeys(
{ token = "", request = defaultRequest } = { request: defaultRequest }
{ token = "", request = defaultRequest, cache = { id: "", keys: [] } } = { request: defaultRequest }
) {
const { data } = await request("GET /meta/public_keys/copilot_api", {
headers: token
? {
Authorization: `token ${token}`,
}
: {},
const headers = token
? {
Authorization: `token ${token}`,
}
: {};

if (cache.id) headers["If-None-Match"] = cache.id;

const response = await request("GET /meta/public_keys/copilot_api", {
headers,
});

return data.public_keys;
if (response.status === 304) {
return { cacheId: cache.id, keys: cache.keys };
}

const cacheId = response.headers.etag || "";
return { cacheId, keys: response.data.public_keys };
}

/** @type {import('..').VerifyRequestByKeyIdInterface} */
Expand All @@ -49,7 +58,7 @@ export async function verifyRequestByKeyId(
assertValidString(keyId, "Invalid keyId");

// receive valid public keys from GitHub
const keys = await fetchVerificationKeys(requestOptions);
const { cacheId, keys } = await fetchVerificationKeys(requestOptions);

// verify provided key Id
const publicKey = keys.find((key) => key.key_identifier === keyId);
Expand All @@ -67,7 +76,8 @@ export async function verifyRequestByKeyId(
throw keyNotFoundError;
}

return verifyRequest(rawBody, signature, publicKey.key);
const isValid = await verifyRequest(rawBody, signature, publicKey.key);
return { isValid, cacheId };
}

function assertValidString(value, message) {
Expand Down
9 changes: 6 additions & 3 deletions test/verification.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ test("verifyRequestByKeyId()", async (t) => {
request: testRequest,
});

t.deepEqual(result, true);
t.deepEqual(result, { isValid: true, cacheId: "" });
});

test("verifyRequestByKeyId() - invalid arguments", async (t) => {
Expand Down Expand Up @@ -132,7 +132,7 @@ test("verifyRequest() - invalid", async (t) => {
t.deepEqual(result, false);
});

test("fetchVerificationKeys()", async (t) => {
test("fetchVerificationKeys() - without cache", async (t) => {
const mockAgent = new MockAgent();
function fetchMock(url, opts) {
opts ||= {};
Expand Down Expand Up @@ -180,5 +180,8 @@ test("fetchVerificationKeys()", async (t) => {
request: testRequest,
});

t.deepEqual(result, publicKeys);
t.deepEqual(result, { cacheId: "", keys: publicKeys });
});

// Up next: test the cache
// test("fetchVerificationKeys() - with cache", async (t) => { });