extension icon indicating copy to clipboard operation
extension copied to clipboard

Add x-api-key header to avoid rate limiting

Open markmhendrickson opened this issue 2 years ago • 10 comments

See relevant info here from @zone117x

markmhendrickson avatar May 23 '22 12:05 markmhendrickson

@zone117x can you help me understand all the changes I need to make here? Am I just adding:

'x-api-key': 'aF8BAm9P37EhTCVQPA6e3gYP3UI9GO86',

here: https://github.com/hirosystems/stacks-wallet-web/blob/bf5e8f9839c8885fce21de05c0eabae9897a4455/src/app/common/api/wrapped-fetch.ts#L1

fbwoolf avatar May 26 '22 19:05 fbwoolf

All http requests made to the API need the x-api-key header. That function looks like it should be updated, but I'm not familiar with this codebase and I don't know the various methods that perform http requests -- but they all need updated.

@janniks provided an example of how to add the header when using stacks.js functions, see https://github.com/hirosystems/stacks.js/tree/master/packages/network#use-the-built-in-api-key-middleware

There's also usage of the @stacks/blockchain-api-client lib: https://github.com/hirosystems/stacks-wallet-web/blob/c07a910307a70506c8b206e382428e7c8be3eaf8/src/app/store/common/api-clients.ts

That code already has example of middleware, but here's a more specific one which sets a header:

import { Configuration, AccountsApi } from '@stacks/blockchain-api-client';

// Example middleware to specify an http header.
const apiConfig = new Configuration({
  middleware: [{
    pre: (req) => {
      const headers = new Headers(req.init.headers);
      headers.set('x-api-key', $API_KEY);
      req.init.headers = headers;
      return Promise.resolve({ url: req.url, init: req.init });
    }
  }]
});

// Example usage of middleware config.
const accountsApi = new AccountsApi(apiConfig);
const balance = await accountsApi.getAccountStxBalance({
  principal: 'ST000000000000000000002AMW42H',
});

There's a test endpoint explained in https://github.com/hirosystems/devops/issues/932#issuecomment-1127893730 which will return an http error response if the header is not specified. This can be used to help identify areas in this codebase where API http requests are made.

zone117x avatar May 27 '22 14:05 zone117x

Ok, great, thanks for the helpful feedback. All of our api requests are using the headers configured in the wrapped-fetch I posted with fetchApi in the link you posted to .../api-clients.ts

https://github.com/hirosystems/stacks-wallet-web/blob/c07a910307a70506c8b206e382428e7c8be3eaf8/src/app/store/common/api-clients.ts#L78

I'll test it out. Thanks!

fbwoolf avatar May 27 '22 14:05 fbwoolf

Waiting for @zone117x to generate a new API key

andresgalante avatar Jun 06 '22 14:06 andresgalante

The key in the linked issue should be usable, are you running into problems?

zone117x avatar Jun 06 '22 14:06 zone117x

The key in the linked issue should be usable, are you running into problems?

The one we have is working fine. Are we ok to merge this if we tested it with the test key and endpoint? Right now, this will go unused until the endpoint is live and we get a real api key, correct?

fbwoolf avatar Jun 06 '22 14:06 fbwoolf

Great! Could you provide private builds to devops and API team so we can test and ensure our backend is working as expected?

We shouldn't merge / deploy this change to production because the new http header will likely cause CORs issues with current production API endpoint. I'll get back to you when we're ready for that.

zone117x avatar Jun 06 '22 14:06 zone117x

Great! Could you provide private builds to devops and API team so we can test and ensure our backend is working as expected?

What is the best way to do this so you have the env variables? Do you just want me to upload a zip file somewhere? Also, do you want it using the strict key and endpoint?

fbwoolf avatar Jun 06 '22 16:06 fbwoolf

@zone117x any feedback from the test build I sent you? Things seem ok?

fbwoolf avatar Jun 22 '22 16:06 fbwoolf

@fbwoolf so far this looks good from light auditing of requests on client-side. Could you create a build using the strict kong endpoint (and associated key) for QA and my own testing? It's designed to reject requests that are missing the key.

zone117x avatar Jun 23 '22 15:06 zone117x