search

semuxproject / semux-core

Semux Core
https://www.semux.org
MIT License
76 stars 32 forks source link

API: make public API endpoints easy to setup #271

Open witoldsz opened 4 years ago

witoldsz commented 4 years ago

🚀 Feature Request

In order to make public API endpoints easy to set up, we must do everything to:

Now there are two public servers providing Semux API:

Both use very different approach and software:

These projects were created because Semux Core is missing dedicated endpoint serving nothing but public API, where one:

Current solution

In my opinion, the API: divide APIs into groups #267 is not a good solution:

Suggestions

  1. rollback the GET=public rule
  2. make it so that once we setup reverse proxy, there is nothing but public API with no wallet endpoints, no authentication sections, etc…

We can either:

The first approach has an advantage that one can publish public API (e.g. reverse proxy points to host:port/api/public) and still use wallet API internally.

The latter approach is implemented in https://github.com/witoldsz/api.semux.online by filtering the swagger specification file. @orogvany said:

Swagger has API so no manual filtering necessary

witoldsz commented 4 years ago

I would like to mention that in my case: special flag to enable only public API would be a little disadvantage, because my public endpoints are also my wallet API playground (I do bypass reverse proxy to access it).

semuxgo commented 4 years ago

I agree that the allowGetMethodsOnly property is causing confusion.

PR #267 was originally designed to divide APIs into logic groups (e.g. wallet-related, tools, chain queries). @orogvany mentioned a requirement of public vs private groups. We kind of mixed these two types of categorization, which leads to the current state of API.

I'd propose we make the following changes.

witoldsz commented 4 years ago

Allow people to open arbitrary combination of APIs through configuration. e.g. api.serviceEnabled=chain,account,delegate,tools.

What would be the reason anyone would like anything else than these two options?

I think it would be easier just to have these two to choose from (less is sometimes the better).

Also, I would also like to lobby the possibility to have public API available WHILE still having an option to use wallet API. Remember, that when running Semux on the server the API is THE ONLY POSSIBLE way to interact with the wallet.

That means, if all we have is to choose from exposing everything or just the public endpoints, then any wallet operations of that Semux instance is not reachable (no GUI, no API).

semuxgo commented 4 years ago

Well, was trying to make the configuration generic. For example, one user may want to disable the Node-related APIs as they expose information about the server.

And I agree that disabling APIs would cause some inconvenience. Like you said, someone might want to expose a public API while keeping the wallet API, presumably protected by the basic authorization.

witoldsz commented 4 years ago

I really do believe that wallet (protected) vs public (not protected) is all we need. It's the golden mean between too much and too little. Flexible enough and easy to figure out. Best if we could have both enabled and still just point the reverse proxy at the public endpoint with trimmed docs, no authentication.

semuxgo commented 4 years ago

This pattern of configuration is very common, not too much. Ethereum Parity takes similar approach. See the [rpc] section of https://wiki.parity.io/Configuring-Parity-Ethereum#example-configtoml.

I still think dividing the API into logical groups is the right direction to go. The reason we added onlyAllowGetMethods was a coincident as we realized almost all get APIs are safe to public access. (I'm going to remove this anyway).

As for backward compatibility, all changes are backward compatible, except:

honeycrypto commented 4 years ago

@semuxgo so the proposed config will be like 

api.enabled = true
api.username = secret1
api.password = secret2

api.public = account, blockchain, delegate, tool
# any non public is protected by default even if not mentioned below 
api.protected = wallet, node

I'd probably prefer to close all protected endpoints by port to be sure that no one has access to them except of localhost, but as tradeoff they can be blacklisted by path on Nginx side.

witoldsz commented 4 years ago

I'd probably prefer to close all protected endpoints by port to be sure that no one has access to them except of localhost, but as tradeoff they can be blacklisted by path on Nginx side.

It would be so much easier to black or white-list endpoint categories if these categories were actually the prefixes. The account category has all the endpoints starting from /account, but all the others do not, e.g.: wallet category has: /accounts, /create-account, /transaction/call, /transaction/vote and others and blockchain has i.a. /transaction or /transaction-result, so /transaction belongs to blockchain while /transaction/call to wallet.

This is really really not proxy-setup-friendly when one has to enumerate dozens of endpoints like these, not to mention upgrades of the API.

What about:

instead of