API: enhance the service grouping #273
Conversation
| @Override | ||
| public boolean isAuthRequired(HttpMethod method, String path) { | ||
| Route route = matchRoute(method, path); | ||
| return route == null ? false : !route.isPublic; |
There was a problem hiding this comment.
return route != null && !route.isPublic is it better?
| private Route matchRoute(HttpMethod method, String path) { | ||
| Route route = null; | ||
| Matcher m = VERSIONED_PATH.matcher(path); | ||
| if (m.matches() && routes.containsKey(ApiVersion.of(m.group(1)))) { |
There was a problem hiding this comment.
named groups would be less confusing than some 1 and 2…
|
So, the concept here does not cover the case where I would like to use my node as a public Semux API node and still be able to access wallet API, is that right? If so, that means the public Semux nodes are, in practice, not accessible for owners (no GUI, no API). |
|
I think you can do both now. The private APIs (wallet) are enabled and protected by basic auth, while the public APIs are just open to public, without basic auth. |
|
OK, but I would feel uncomfortable exposing everything and having just the auth as the final shield from accessing my wallet. Of course I could set up some extended matching rules, not to allow certain endpoints to go through (black/white listed in reverse proxy service), but it complicates the deployment. Last, but not least, when exposing public API I would like the swagger definition to be trimmed down to only what's available. See the https://api.semux.online. You won't find wallet endpoints there or the authentication schemes definition. And I can still see and use full version when bypassing the proxy. |
|
+1 for the dynamic swagger page, but what will be the best implementation for this? Desktop users want to see all available endpoints, even the protected ones (because they are on localhost). While public API providers want to show only public endpoints in swagger UI.
TL;DR: i'm ok with exposing to internet password protected endpoints. Dynamic Swagger UI is good to have to minimise required configs to proxy/firewall and certificates. |
|
Dynamic swagger generation would require extra engineering work. Will consider this in another PR. Displaying all APIs (even though some of them are inaccessible) won't necessarily be a bad thing. I personally use https://api.semux.online not only for calling these endpoints but also for reading the API specification. |
Description
This PR introduces the concept of public and private services, to partially resolve #271.
Test Plan
Existing unit tests
Related issues and/or PRs
#271