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

Enable specifying api key in config #18

Merged
merged 6 commits into from
Aug 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion packages/local/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
"scripts": {
"postinstall": "cd registry && npm install",
"test": "mocha",
"test:ci": "npm run build && mocha --exit",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not related to this PR, but lerna and nx handle the need to build (or not) before running the test. Having this written into the test command was causing issues.

"test:ci": "mocha --exit",
"lint": "eslint '**/*.{js,ts}'",
"lint:fix": "npm run lint -- --fix",
"prepublishOnly": "npm run build",
Expand Down
3 changes: 2 additions & 1 deletion packages/sdk/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@
},
"scripts": {
"prepublishOnly": "npm run build",
"test": "npm run test:browser && mocha --exit",
"test": "npm run test:browser && npm run test:mocha",
"test:mocha": "mocha --exit",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this command to make it easier for devs to just run the mocha tests since the browser tests take so long.

"test:browser": "npx playwright install --with-deps && PW_TS_ESM_ON=true playwright test",
"test:browser-show-report": "npx playwright show-report",
"test:ci": "npm run coverage",
Expand Down
15 changes: 15 additions & 0 deletions packages/sdk/src/helpers/config.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import fs from "node:fs";
import { type WaitableTransactionReceipt } from "../registry/utils.js";
import { type FetchConfig } from "../validator/client/index.js";
import { type ChainName, getBaseUrl } from "./chains.js";
import { type Signer, type ExternalProvider, getSigner } from "./ethers.js";

export interface ReadConfig {
baseUrl: string;
aliases?: AliasesNameMap;
apiKey?: string;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The usage is now:

new Validator({ apiKey, ... });
new Database({ apiKey, ... });

}

export interface SignerConfig {
Expand Down Expand Up @@ -98,3 +100,16 @@ export function jsonFileAliases(filepath: string): AliasesNameMap {
},
};
}

export function prepReadConfig(config: Partial<ReadConfig>): FetchConfig {
const conf: FetchConfig = {};
if (config.apiKey) {
conf.init = {
headers: {
"Api-Key": config.apiKey,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the bulk of the logic. Take the apiKey param and set the Api-Key request header with the given value.

},
};
}

return { ...config, ...conf };
}
1 change: 1 addition & 0 deletions packages/sdk/src/helpers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export {
extractChainId,
extractSigner,
jsonFileAliases,
prepReadConfig,
} from "./config.js";
export {
type Signer,
Expand Down
2 changes: 1 addition & 1 deletion packages/sdk/src/registry/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ export async function extractReadonly(
): Promise<ReadConfig> {
const [{ chainId }] = await validateTables({ tables, type });
const baseUrl = await extractBaseUrl(conn, chainId);
return { baseUrl };
return { baseUrl, apiKey: conn.apiKey };
}

/**
Expand Down
3 changes: 2 additions & 1 deletion packages/sdk/src/validator/client/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Fetcher } from "./fetcher.js";
import { ApiResponse, ApiError, FetchConfig } from "./types.js";
import type { paths as Paths, components as Components } from "./validator.js";
import { prepReadConfig } from "../../helpers/index.js";

export { ApiResponse, Fetcher, ApiError, FetchConfig };
export type { Paths, Components };
Expand All @@ -9,6 +10,6 @@ export function getFetcher(
config: FetchConfig
): ReturnType<typeof Fetcher.for<Paths>> {
const fetcher = Fetcher.for<Paths>();
fetcher.configure(config);
fetcher.configure(prepReadConfig(config));
return fetcher;
}
2 changes: 1 addition & 1 deletion packages/sdk/src/validator/query.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { type Signal } from "../helpers/await.js";
import { type Signal } from "../helpers/index.js";
import { type FetchConfig, type Paths, getFetcher } from "./client/index.js";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated cleanup

import { hoistApiError } from "./errors.js";

Expand Down
9 changes: 6 additions & 3 deletions packages/sdk/test/browser/server/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

50 changes: 49 additions & 1 deletion packages/sdk/test/database.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
/* eslint-disable @typescript-eslint/no-non-null-assertion */
import assert, { deepStrictEqual, strictEqual, rejects, match } from "assert";
import assert, {
deepStrictEqual,
equal,
strictEqual,
rejects,
match,
} from "assert";
import { describe, test } from "mocha";
import { getAccounts } from "@tableland/local";
import { getDefaultProvider } from "ethers";
import { Database } from "../src/database.js";
import { Statement } from "../src/statement.js";
import { getAbortSignal } from "../src/helpers/await.js";
import { getDelay, getRange } from "../src/helpers/utils.js";
import {
TEST_TIMEOUT_FACTOR,
TEST_PROVIDER_URL,
Expand Down Expand Up @@ -680,4 +687,45 @@ describe("database", function () {
});
});
});

describe("rate limit", function () {
test("when too many read calls are sent", async function () {
await rejects(
Promise.all(
getRange(15).map(async () => {
return await db.prepare("select * from healthbot_31337_1;").all();
})
),
(err: any) => {
strictEqual(err.message, "ALL_ERROR: Too Many Requests");
return true;
}
);

// need to ensure the following tests aren't affected by validator throttling
await getDelay(2000);
});

test("when valid api key is used there is no limit on read queries", async function () {
const apiKey = "foo";
const db = new Database({
signer,
autoWait: true,
apiKey,
});
const responses = await Promise.all(
getRange(15).map(async () => {
return await db.prepare("select * from healthbot_31337_1;").all();
})
);

// need to ensure the following tests aren't affected by validator throttling
await getDelay(2000);

equal(responses.length, 15);
for (const res of responses) {
deepStrictEqual(res.results, [{ counter: 1 }]);
}
});
});
});
41 changes: 41 additions & 0 deletions packages/sdk/test/validator.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* eslint-disable @typescript-eslint/no-non-null-assertion */
import assert, {
strictEqual,
equal,
rejects,
match,
notStrictEqual,
Expand Down Expand Up @@ -450,5 +451,45 @@ describe("validator", function () {
);
await getDelay(5000);
});

test("when valid api key is used there is no limit on calls to `health()`", async function () {
const apiKey = "foo";
const api = new Validator({
baseUrl,
apiKey,
});
const responses = await Promise.all(
getRange(15).map(async () => await api.health())
);

equal(responses.length, 15);
equal(
responses.every((r: unknown) => r === true),
true
);
});

test("when valid api key is used there is no limit on calls to `version()`", async function () {
const apiKey = "foo";
const api = new Validator({
baseUrl,
apiKey,
});
const responses = await Promise.all(
getRange(15).map(async () => await api.version())
);

equal(responses.length, 15);
for (const res of responses) {
deepStrictEqual(res, {
binaryVersion: "n/a",
buildDate: "n/a",
gitBranch: "n/a",
gitCommit: "n/a",
gitState: "n/a",
gitSummary: "n/a",
});
}
});
});
});
3 changes: 2 additions & 1 deletion packages/sdk/test/validator/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
"HTTP": {
"Port": "8081",
"RateLimInterval": "1s",
"MaxRequestPerInterval": 10
"MaxRequestPerInterval": 10,
"APIKey": "foo"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping the tests simple for the time being. The api key is always going to be "foo"

As an aside in regard to the api key implementation in the validator... Since the key is a fixed config value we should probably ensure the value is only ever used on the server side of the Studio.
Otherwise anyone who logs into the Studio will be able to get the api key and use it as much as they like.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vercel makes it hard to expose secrets to the client. By default all env vars are server side only. You have to opt in to exposing them to the client by prefixing the env var name with NEXT_PUBLIC_.

},
"Gateway": {
"ExternalURIPrefix": "http://localhost:8081",
Expand Down
Loading