bridge-server icon indicating copy to clipboard operation
bridge-server copied to clipboard

`api_key` parameter is mis-named

Open Mr0grog opened this issue 7 years ago • 8 comments

The documentation for bridge server notes that if the server is configured with an api_key option, a corresponding parameter named api_key must be included with all requests (https://github.com/stellar/bridge-server/blob/master/readme_bridge.md#config).

However, the actual parameter in the request is apiKey (https://github.com/stellar/bridge-server/blob/master/src/github.com/stellar/gateway/server/middlewares.go#L42).

Not sure whether the documentation just needs to change to reflect reality or the parameter name should change. In an ideal world, it seems like the latter would be better (so it is in keeping with the naming convention for all other params), but if anyone is using it, that could easily break them. Maybe accept both?

Mr0grog avatar Sep 01 '16 17:09 Mr0grog

I don't think anyone is using it. I'll leave it to scott to decide which way.

jedmccaleb avatar Sep 01 '16 18:09 jedmccaleb

Relatedly, is there a reason why using it is not recommended? https://github.com/stellar/bridge-server/blob/master/readme_bridge.md#security

Mr0grog avatar Sep 01 '16 18:09 Mr0grog

Relatedly, is there a reason why using it is not recommended? https://github.com/stellar/bridge-server/blob/master/readme_bridge.md#security

Access should be configured at the proxy/load balancer level rather than by using api_key param.

bartekn avatar Sep 01 '16 18:09 bartekn

I need to do some thinking about what we should do with that code in general. Having a simple bearer token be our answer to securing your internal communications makes me very uncomfortable; IMO we should be doing a better job, at the very least we should be using a protocol that doesn't transmit secret information over the network.

We should be aiming to provide defense in depth when people install our software in their infrastructure.


As far as naming the parameter, it should be api_key. The rest of that config file is snake cased.

nullstyle avatar Sep 01 '16 18:09 nullstyle

Access should be configured at the proxy/load balancer level rather than by using api_key param

Ok, so this is really a concern that the option to do it will encourage people not to bother with additional better methods (e.g. securing the network, having some other auth server that implements their own security, etc.)?

Having a simple bearer token be our answer to securing your internal communications makes me very uncomfortable; IMO we should be doing a better job

Totally agree! Just seemed surprising to effectively say "we don't have a great security solution for you, so you really shouldn't even use the middling solution we at least do have." That is, at least, how that paragraph read to me. I guess I didn't read the bit about setting the server up in an isolated environment as somehow being mutually exclusive from having an API key.

Mr0grog avatar Sep 01 '16 18:09 Mr0grog

The rest of that config file is snake cased.

Just to be clear, the one in the config file is properly snake cased. It’s the one that is received in requests that is not.

Mr0grog avatar Sep 01 '16 18:09 Mr0grog

ah, I didn't realize. I'll look into it and make a call tomorrow.

nullstyle avatar Sep 01 '16 21:09 nullstyle

The documentation for bridge server notes that if the server is configured with an api_key option, a corresponding parameter named api_key must be included with all requests (https://github.com/stellar/bridge-server/blob/master/readme_bridge.md#config).

However, the actual parameter in the request is apiKey (https://github.com/stellar/bridge-server/blob/master/src/github.com/stellar/gateway/server/middlewares.go#L42).

Not sure whether the documentation just needs to change to reflect reality or the parameter name should change. In an ideal world, it seems like the latter would be better (so it is in keeping with the naming convention for all other params), but if anyone is using it, that could easily break them. Maybe accept both?

Was there any final decision on which option to take? the parameter is still called apiKey in code but the documentation is still called api_key. After all, thank you very much for making our work easy for those of us who want to build in the stellar network!

theclabs avatar Jul 19 '19 04:07 theclabs