feat: change handler to '/' for external users, use only rpc user name to choose queue

Author: knst

src/httprpc.cpp

@@ -176,14 +176,6 @@ static bool HTTPReq_JSONRPC(const CoreContext& context, HTTPRequest* req, bool e
         return false;
     }
 
-    if (jreq.authUser == gArgs.GetArg("-rpcexternaluser", "") && !jreq.authUser.empty()) {
-        if (!external) {
-            LogPrintf("RPC User '%s' is allowed to call rpc only by path /external\n", jreq.authUser);
-            req->WriteReply(HTTP_FORBIDDEN);
-            return false;
-        }
-        LogPrintf("RPC user '%s' is external\n", jreq.authUser);
-    }
     try {
         // Parse request
         UniValue valRequest;
@@ -306,12 +298,10 @@ bool StartHTTPRPC(const CoreContext& context)
         return false;
 
     auto handle_rpc = [&context](HTTPRequest* req, const std::string&) { return HTTPReq_JSONRPC(context, req); };
-    auto handle_rpc_external = [&context](HTTPRequest* req, const std::string&) { return HTTPReq_JSONRPC(context, req, true); };
-    RegisterHTTPHandler("/", true, false, handle_rpc);
+    RegisterHTTPHandler("/", true, handle_rpc);
     if (g_wallet_init_interface.HasWalletSupport()) {
-        RegisterHTTPHandler("/wallet/", false, false, handle_rpc);
+        RegisterHTTPHandler("/wallet/", false, handle_rpc);
     }
-    RegisterHTTPHandler("/external", true, true, handle_rpc_external);
     struct event_base* eventBase = EventBase();
     assert(eventBase);
     httpRPCTimerInterface = std::make_unique<HTTPRPCTimerInterface>(eventBase);
@@ -327,7 +317,6 @@ void InterruptHTTPRPC()
 void StopHTTPRPC()
 {
     LogPrint(BCLog::RPC, "Stopping HTTP RPC server\n");
-    UnregisterHTTPHandler("/external", true);
     UnregisterHTTPHandler("/", true);
     if (g_wallet_init_interface.HasWalletSupport()) {
         UnregisterHTTPHandler("/wallet/", false);

src/httpserver.cpp

@@ -126,13 +126,12 @@ class WorkQueue
 
 struct HTTPPathHandler
 {
-    HTTPPathHandler(std::string _prefix, bool _exactMatch, bool external, HTTPRequestHandler _handler):
-        prefix(_prefix), exactMatch(_exactMatch), m_external(external), handler(_handler)
+    HTTPPathHandler(std::string _prefix, bool _exactMatch, HTTPRequestHandler _handler):
+        prefix(_prefix), exactMatch(_exactMatch), handler(_handler)
     {
     }
     std::string prefix;
     bool exactMatch;
-    bool m_external;
     HTTPRequestHandler handler;
 };
 
@@ -262,14 +261,31 @@ static void http_request_cb(struct evhttp_request* req, void* arg)
             break;
         }
     }
+    const bool is_external_request = [&hreq]() -> bool {
+        const std::string external_username = gArgs.GetArg("-rpcexternaluser", "");
+        if (external_username.empty()) return false;
+
+        const std::string strAuth = hreq->GetHeader("authorization").second;
+        if (strAuth.substr(0, 6) != "Basic ")
+            return false;
+
+        std::string strUserPass64 = TrimString(strAuth.substr(6));
+        bool invalid;
+        std::string strUserPass = DecodeBase64(strUserPass64, &invalid);
+        if (invalid) return false;
+
+        if (strUserPass.find(':') == std::string::npos) return false;
+
+        return strUserPass.substr(0, strUserPass.find(':')) == external_username;
+    }();
 
     // Dispatch to worker thread
     if (i != iend) {
-        auto item{std::make_unique<HTTPWorkItem>(std::move(hreq), path, i->handler)}; /// this handler!
+        auto item{std::make_unique<HTTPWorkItem>(std::move(hreq), path, i->handler)};
         assert(g_work_queue);
 
         // We have queue created only if RPC arg 'rpcexternaluser' is specified
-        if (i->m_external && g_work_queue_external) {
+        if (is_external_request && g_work_queue_external) {
             if (g_work_queue_external->Enqueue(item.get())) {
                 item.release();
             } else {
@@ -673,10 +689,10 @@ HTTPRequest::RequestMethod HTTPRequest::GetRequestMethod() const
     }
 }
 
-void RegisterHTTPHandler(const std::string &prefix, bool exactMatch, bool external, const HTTPRequestHandler &handler)
+void RegisterHTTPHandler(const std::string &prefix, bool exactMatch, const HTTPRequestHandler &handler)
 {
-    LogPrint(BCLog::HTTP, "Registering HTTP handler for %s (exactmatch %d external %d)\n", prefix, exactMatch, external);
-    pathHandlers.push_back(HTTPPathHandler(prefix, exactMatch, external, handler));
+    LogPrint(BCLog::HTTP, "Registering HTTP handler for %s (exactmatch %d)\n", prefix, exactMatch);
+    pathHandlers.push_back(HTTPPathHandler(prefix, exactMatch, handler));
 }
 
 void UnregisterHTTPHandler(const std::string &prefix, bool exactMatch)

src/httpserver.h

@@ -41,7 +41,7 @@ typedef std::function<bool(HTTPRequest* req, const std::string &)> HTTPRequestHa
  * If multiple handlers match a prefix, the first-registered one will
  * be invoked.
  */
-void RegisterHTTPHandler(const std::string &prefix, bool exactMatch, bool external, const HTTPRequestHandler &handler);
+void RegisterHTTPHandler(const std::string &prefix, bool exactMatch, const HTTPRequestHandler &handler);
 /** Unregister handler for prefix */
 void UnregisterHTTPHandler(const std::string &prefix, bool exactMatch);
 

src/rest.cpp

@@ -723,7 +723,7 @@ void StartREST(const CoreContext& context)
 {
     for (const auto& up : uri_prefixes) {
         auto handler = [&context, up](HTTPRequest* req, const std::string& prefix) { return up.handler(context, req, prefix); };
-        RegisterHTTPHandler(up.prefix, false, false, handler);
+        RegisterHTTPHandler(up.prefix, false, handler);
     }
 }
 

test/functional/rpc_platform_filter.py

@@ -43,18 +43,12 @@ def run_test(self):
         url = urllib.parse.urlparse(self.nodes[0].url)
 
         def test_command(method, params, auth, expected_status, should_not_match=False):
-            test_command_helper(method, params, '/', auth, expected_status, should_not_match)
-
-        def test_external_command(method, params, auth, expected_status, should_not_match=False):
-            test_command_helper(method, params, '/external', auth, expected_status, should_not_match)
-
-        def test_command_helper(method, params, path, auth, expected_status, should_not_match):
             conn = http.client.HTTPConnection(url.hostname, url.port)
             conn.connect()
             body = {"method": method}
             if len(params):
                 body["params"] = params
-            conn.request('POST', path, json.dumps(body), {"Authorization": "Basic " + str_to_b64str(auth)})
+            conn.request('POST', '/', json.dumps(body), {"Authorization": "Basic " + str_to_b64str(auth)})
             resp = conn.getresponse()
             if should_not_match:
                 assert resp.status != expected_status
@@ -120,15 +114,15 @@ def test_command_helper(method, params, path, auth, expected_status, should_not_
         test_command("debug", ["1"], rpcuser_authpair_operator, 200)
 
 
-        self.log.info("Restart node with /external handler...")
-        test_external_command("getbestblockhash", [], rpcuser_authpair_platform, 200)
-        test_external_command("getblockchaininfo", [], rpcuser_authpair_platform, 403)
-
+        self.log.info("Restart node with -rpcexternaluser...")
         self.restart_node(0, extra_args=["-rpcexternaluser=platform-user"])
-        test_command("getbestblockhash", [], rpcuser_authpair_platform, 403)
-        test_external_command("getbestblockhash", [], rpcuser_authpair_platform, 200)
-        test_external_command("getblockchaininfo", [], rpcuser_authpair_platform, 403)
-        test_external_command("getbestblockhash", [], rpcuser_authpair_operator, 200)
+
+        external_log_str = "HTTP: Calling handler for external user"
+        expected_log_str = "ThreadRPCServer method="
+        with self.nodes[0].assert_debug_log(expected_msgs=[expected_log_str, external_log_str]):
+            test_command("getbestblockhash", [], rpcuser_authpair_platform, 200)
+        with self.nodes[0].assert_debug_log(expected_msgs=[expected_log_str], unexpected_msgs = [external_log_str]):
+            test_command("getbestblockhash", [], rpcuser_authpair_operator, 200)