Enable specifying api key in config #18
Conversation
| @@ -45,7 +45,7 @@ | |||
| "scripts": { | |||
| "postinstall": "cd registry && npm install", | |||
| "test": "mocha", | |||
| "test:ci": "npm run build && mocha --exit", | |||
There was a problem hiding this comment.
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.
| @@ -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", | |||
There was a problem hiding this comment.
I added this command to make it easier for devs to just run the mocha tests since the browser tests take so long.
| 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; |
There was a problem hiding this comment.
The usage is now:
new Validator({ apiKey, ... });
new Database({ apiKey, ... });
| if (config.apiKey) { | ||
| conf.init = { | ||
| headers: { | ||
| "Api-Key": config.apiKey, |
There was a problem hiding this comment.
This is the bulk of the logic. Take the apiKey param and set the Api-Key request header with the given value.
| @@ -1,4 +1,4 @@ | |||
| import { type Signal } from "../helpers/await.js"; | |||
| import { type Signal } from "../helpers/index.js"; | |||
There was a problem hiding this comment.
unrelated cleanup
| @@ -3,7 +3,8 @@ | |||
| "HTTP": { | |||
| "Port": "8081", | |||
| "RateLimInterval": "1s", | |||
| "MaxRequestPerInterval": 10 | |||
| "MaxRequestPerInterval": 10, | |||
| "APIKey": "foo" | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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_.
|
Is this still WIP @joewagner? |
It's ready for review, but it's aiming at the cleanup branch, which I'll merge and then point this at main. |
There was a problem hiding this comment.
This looks great. Nice work. I'm fine with this as is, but just want to surface an idea... What about making the config option called extraHeaders and it's type be { [key: string]: string }. I'm just thinking this is, at this point at least, kind of a bandaid fix for Studio and not a real feature we want to advertise. If we leave the config option as kinda generic like that, we don't have to explain much about it in the docs.
This enables specifying an api key that bypasses the Validator's rate limiting.
You can pass an
apiKeyparameter to theValidatorandDatabaseclasses, which is converted to the correct header and send with read requests.Example use: