Skip to content
This repository has been archived by the owner on Sep 12, 2019. It is now read-only.

api_key parameter is mis-named #27

Open
Mr0grog opened this issue Sep 1, 2016 · 8 comments
Open

api_key parameter is mis-named #27

Mr0grog opened this issue Sep 1, 2016 · 8 comments
Assignees
Milestone

Comments

@Mr0grog
Copy link
Contributor

Mr0grog commented Sep 1, 2016

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?

@jedmccaleb
Copy link
Contributor

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

@Mr0grog
Copy link
Contributor Author

Mr0grog commented Sep 1, 2016

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

@bartekn
Copy link
Contributor

bartekn commented Sep 1, 2016

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.

@nullstyle
Copy link
Contributor

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.

@Mr0grog
Copy link
Contributor Author

Mr0grog commented Sep 1, 2016

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
Copy link
Contributor Author

Mr0grog commented Sep 1, 2016

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.

@nullstyle
Copy link
Contributor

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

@jedmccaleb jedmccaleb added this to the after port milestone Oct 3, 2017
@theclabs
Copy link

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!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants