From 713b0063b73dad395525b4515aa2787a144c272f Mon Sep 17 00:00:00 2001 From: heguanhui Date: Fri, 23 Jan 2026 17:03:18 +0800 Subject: [PATCH 1/4] [Fix](restapi) fix node action restapi throw '-parameters' error because of PathVariable annotation without value declaration (#59708) --- .../org/apache/doris/httpv2/rest/manager/NodeAction.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/manager/NodeAction.java b/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/manager/NodeAction.java index 4067864dc30219..7e24078934570f 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/manager/NodeAction.java +++ b/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/manager/NodeAction.java @@ -611,8 +611,8 @@ public Object setConfigBe(HttpServletRequest request, HttpServletResponse respon } @PostMapping("/{action}/be") - public Object operateBackend(HttpServletRequest request, HttpServletResponse response, @PathVariable String action, - @RequestBody BackendReqInfo reqInfo) { + public Object operateBackend(HttpServletRequest request, HttpServletResponse response, + @PathVariable("action") String action, @RequestBody BackendReqInfo reqInfo) { try { if (needRedirect(request.getScheme())) { return redirectToHttps(request); @@ -659,7 +659,7 @@ public Object operateBackend(HttpServletRequest request, HttpServletResponse res @PostMapping("/{action}/fe") public Object operateFrontends(HttpServletRequest request, HttpServletResponse response, - @PathVariable String action, @RequestBody FrontendReqInfo reqInfo) { + @PathVariable("action") String action, @RequestBody FrontendReqInfo reqInfo) { try { if (needRedirect(request.getScheme())) { return redirectToHttps(request); @@ -691,7 +691,7 @@ public Object operateFrontends(HttpServletRequest request, HttpServletResponse r @PostMapping("/{action}/broker") public Object operateBroker(HttpServletRequest request, HttpServletResponse response, - @PathVariable String action, @RequestBody BrokerReqInfo reqInfo) { + @PathVariable("action") String action, @RequestBody BrokerReqInfo reqInfo) { try { if (!Env.getCurrentEnv().isMaster()) { return redirectToMasterOrException(request, response); From ab91660eea3021976d1a2f6ab9bea72726135d3b Mon Sep 17 00:00:00 2001 From: guoqiang Date: Tue, 30 Jun 2026 18:28:58 +0800 Subject: [PATCH 2/4] [fix](auth) add authentication and authorization for manager node and query qerror REST APIs The node management endpoints (POST /rest/v2/manager/node/{action}/{fe,be,broker}) allowed adding or dropping cluster nodes without any authentication or authorization. Add executeCheckPassword + checkAdminAuth so they require an authenticated ADMIN user, consistent with set_config/fe and set_config/be. GET /rest/v2/manager/query/qerror/{id} (getStats) had neither authentication nor authorization: its signature took no request/response and the global AuthInterceptor only covers /rest/v1/**, so it was reachable anonymously even with enable_all_http_auth=true. Add executeCheckPassword and checkAuthByUserAndQueryId, matching the /profile and /trace_id endpoints, so a non-admin can only read their own query stats. Add a p0 regression test covering both gaps. --- .../doris/httpv2/rest/manager/NodeAction.java | 6 ++ .../rest/manager/QueryProfileAction.java | 10 +- .../auth_p0/test_http_node_action_auth.groovy | 102 ++++++++++++++++++ 3 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 regression-test/suites/auth_p0/test_http_node_action_auth.groovy diff --git a/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/manager/NodeAction.java b/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/manager/NodeAction.java index 7e24078934570f..78f79057d35871 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/manager/NodeAction.java +++ b/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/manager/NodeAction.java @@ -613,6 +613,8 @@ public Object setConfigBe(HttpServletRequest request, HttpServletResponse respon @PostMapping("/{action}/be") public Object operateBackend(HttpServletRequest request, HttpServletResponse response, @PathVariable("action") String action, @RequestBody BackendReqInfo reqInfo) { + ActionAuthorizationInfo authInfo = executeCheckPassword(request, response); + checkAdminAuth(authInfo.userIdentity); try { if (needRedirect(request.getScheme())) { return redirectToHttps(request); @@ -660,6 +662,8 @@ public Object operateBackend(HttpServletRequest request, HttpServletResponse res @PostMapping("/{action}/fe") public Object operateFrontends(HttpServletRequest request, HttpServletResponse response, @PathVariable("action") String action, @RequestBody FrontendReqInfo reqInfo) { + ActionAuthorizationInfo authInfo = executeCheckPassword(request, response); + checkAdminAuth(authInfo.userIdentity); try { if (needRedirect(request.getScheme())) { return redirectToHttps(request); @@ -692,6 +696,8 @@ public Object operateFrontends(HttpServletRequest request, HttpServletResponse r @PostMapping("/{action}/broker") public Object operateBroker(HttpServletRequest request, HttpServletResponse response, @PathVariable("action") String action, @RequestBody BrokerReqInfo reqInfo) { + ActionAuthorizationInfo authInfo = executeCheckPassword(request, response); + checkAdminAuth(authInfo.userIdentity); try { if (!Env.getCurrentEnv().isMaster()) { return redirectToMasterOrException(request, response); diff --git a/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/manager/QueryProfileAction.java b/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/manager/QueryProfileAction.java index 7af8a05b648d85..884c4d95cf6351 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/manager/QueryProfileAction.java +++ b/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/manager/QueryProfileAction.java @@ -372,7 +372,15 @@ private String getQueryIdByTraceIdImpl(HttpServletRequest request, String traceI * Query qError. */ @RequestMapping(path = "/qerror/{id}", method = RequestMethod.GET) - public ResponseEntity getStats(@PathVariable(value = "id") String id) { + public Object getStats(HttpServletRequest request, HttpServletResponse response, + @PathVariable(value = "id") String id) { + executeCheckPassword(request, response); + try { + checkAuthByUserAndQueryId(id); + } catch (AuthenticationException e) { + return ResponseEntityBuilder.badRequest(e.getMessage()); + } + ProfileElement profile = ProfileManager.getInstance().findProfileElementObject(id); if (profile == null) { return ResponseEntityBuilder.notFound(null); diff --git a/regression-test/suites/auth_p0/test_http_node_action_auth.groovy b/regression-test/suites/auth_p0/test_http_node_action_auth.groovy new file mode 100644 index 00000000000000..6ddc95ffe895a9 --- /dev/null +++ b/regression-test/suites/auth_p0/test_http_node_action_auth.groovy @@ -0,0 +1,102 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +import org.junit.Assert; + +// Verify the node management endpoints (add/drop fe/be/broker) require +// authentication and ADMIN privilege. Without the check, any caller could +// add or drop cluster nodes via these REST APIs. +suite("test_http_node_action_auth", "p0,auth,nonConcurrent") { + String suiteName = "test_http_node_action_auth" + String user = "${suiteName}_user" + String pwd = 'C123_567p' + try_sql("DROP USER ${user}") + sql """CREATE USER '${user}' IDENTIFIED BY '${pwd}'""" + + try { + sql """ ADMIN SET ALL FRONTENDS CONFIG ("enable_all_http_auth" = "true"); """ + + def operateFe = { check_func -> + httpTest { + basicAuthorization "${user}", "${pwd}" + endpoint "${context.config.feHttpAddress}" + uri "/rest/v2/manager/node/ADD/fe" + op "post" + body """{"role": "OBSERVER", "hostPort": "127.0.0.1:9010"}""" + check check_func + } + } + + def operateBe = { check_func -> + httpTest { + basicAuthorization "${user}", "${pwd}" + endpoint "${context.config.feHttpAddress}" + uri "/rest/v2/manager/node/ADD/be" + op "post" + body """{"hostPorts": ["127.0.0.1:9050"]}""" + check check_func + } + } + + // A non-admin user must be rejected by the ADMIN privilege check. + operateFe.call() { + respCode, body -> + log.info("add fe (non-admin) body:${body}") + assertTrue("${body}".contains("Unauthorized")) + } + + operateBe.call() { + respCode, body -> + log.info("add be (non-admin) body:${body}") + assertTrue("${body}".contains("Unauthorized")) + } + + sql """grant 'admin' to ${user}""" + + // After granting ADMIN, the request passes the auth check. The add + // operation itself may still fail (fake host), but it must no longer + // be rejected with an authorization error. + operateFe.call() { + respCode, body -> + log.info("add fe (admin) body:${body}") + assertFalse("${body}".contains("Unauthorized")) + } + + operateBe.call() { + respCode, body -> + log.info("add be (admin) body:${body}") + assertFalse("${body}".contains("Unauthorized")) + } + + // The query qerror endpoint must require authentication. Without + // credentials it must not return the stats payload (200 ok). + httpTest { + endpoint "${context.config.feHttpAddress}" + uri "/rest/v2/manager/query/qerror/no_such_query_id" + op "get" + check { + respCode, body -> + log.info("qerror (no auth) respCode:${respCode} body:${body}") + assertTrue(respCode == 401 || "${body}".contains("Unauthorized") + || "${body}".contains("Authentication")) + } + } + } finally { + sql """ ADMIN SET ALL FRONTENDS CONFIG ("enable_all_http_auth" = "false"); """ + try_sql("DROP USER ${user}") + } +} From 28afde24fa94ff726d30f9c123c5e59829e58aff Mon Sep 17 00:00:00 2001 From: guoqiang Date: Tue, 30 Jun 2026 18:41:00 +0800 Subject: [PATCH 3/4] [test](auth) make node action test cluster-safe (no phantom nodes) The admin-positive assertions used ADD with 127.0.0.1 addresses, which on a real (distributed) cluster would not match an existing node and would actually register a phantom FE observer / BE into the editlog with no cleanup, polluting cluster state and risking later tests. Switch the positive path to DROP on RFC 5737 TEST-NET addresses (192.0.2.x), which can never match a real node: it reaches the operation, returns a harmless 'does not exist' error, proves the ADMIN check passed, and mutates nothing. The negative (non-admin) cases keep ADD since the auth check rejects them before the node operation runs. --- .../auth_p0/test_http_node_action_auth.groovy | 47 ++++++++++++------- 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/regression-test/suites/auth_p0/test_http_node_action_auth.groovy b/regression-test/suites/auth_p0/test_http_node_action_auth.groovy index 6ddc95ffe895a9..5b1774a44d0193 100644 --- a/regression-test/suites/auth_p0/test_http_node_action_auth.groovy +++ b/regression-test/suites/auth_p0/test_http_node_action_auth.groovy @@ -20,46 +20,56 @@ import org.junit.Assert; // Verify the node management endpoints (add/drop fe/be/broker) require // authentication and ADMIN privilege. Without the check, any caller could // add or drop cluster nodes via these REST APIs. +// +// NOTE on cluster safety: the bogus node addresses below use the RFC 5737 +// TEST-NET-1 range (192.0.2.0/24), which can never match a real FE/BE in any +// cluster. The negative (non-admin) cases use ADD, but the ADMIN check runs +// before the node operation, so the add is never executed. The positive +// (admin) cases use DROP, which on a non-existent node returns a harmless +// "does not exist" error -- it never mutates real cluster state. suite("test_http_node_action_auth", "p0,auth,nonConcurrent") { String suiteName = "test_http_node_action_auth" String user = "${suiteName}_user" String pwd = 'C123_567p' + String bogusFe = "192.0.2.111:12345" + String bogusBe = "192.0.2.112:12345" try_sql("DROP USER ${user}") sql """CREATE USER '${user}' IDENTIFIED BY '${pwd}'""" try { sql """ ADMIN SET ALL FRONTENDS CONFIG ("enable_all_http_auth" = "true"); """ - def operateFe = { check_func -> + def operateFe = { user_name, password, action, check_func -> httpTest { - basicAuthorization "${user}", "${pwd}" + basicAuthorization "${user_name}", "${password}" endpoint "${context.config.feHttpAddress}" - uri "/rest/v2/manager/node/ADD/fe" + uri "/rest/v2/manager/node/${action}/fe" op "post" - body """{"role": "OBSERVER", "hostPort": "127.0.0.1:9010"}""" + body """{"role": "OBSERVER", "hostPort": "${bogusFe}"}""" check check_func } } - def operateBe = { check_func -> + def operateBe = { user_name, password, action, check_func -> httpTest { - basicAuthorization "${user}", "${pwd}" + basicAuthorization "${user_name}", "${password}" endpoint "${context.config.feHttpAddress}" - uri "/rest/v2/manager/node/ADD/be" + uri "/rest/v2/manager/node/${action}/be" op "post" - body """{"hostPorts": ["127.0.0.1:9050"]}""" + body """{"hostPorts": ["${bogusBe}"]}""" check check_func } } // A non-admin user must be rejected by the ADMIN privilege check. - operateFe.call() { + // The node operation is never reached, so nothing is mutated. + operateFe.call(user, pwd, "ADD") { respCode, body -> log.info("add fe (non-admin) body:${body}") assertTrue("${body}".contains("Unauthorized")) } - operateBe.call() { + operateBe.call(user, pwd, "ADD") { respCode, body -> log.info("add be (non-admin) body:${body}") assertTrue("${body}".contains("Unauthorized")) @@ -67,23 +77,24 @@ suite("test_http_node_action_auth", "p0,auth,nonConcurrent") { sql """grant 'admin' to ${user}""" - // After granting ADMIN, the request passes the auth check. The add - // operation itself may still fail (fake host), but it must no longer - // be rejected with an authorization error. - operateFe.call() { + // After granting ADMIN, the request passes the auth check. We use DROP + // on a bogus (TEST-NET) node so the call reaches the operation but only + // gets a "does not exist" error -- it must no longer be rejected with an + // authorization error, and must not touch any real node. + operateFe.call(user, pwd, "DROP") { respCode, body -> - log.info("add fe (admin) body:${body}") + log.info("drop fe (admin) body:${body}") assertFalse("${body}".contains("Unauthorized")) } - operateBe.call() { + operateBe.call(user, pwd, "DROP") { respCode, body -> - log.info("add be (admin) body:${body}") + log.info("drop be (admin) body:${body}") assertFalse("${body}".contains("Unauthorized")) } // The query qerror endpoint must require authentication. Without - // credentials it must not return the stats payload (200 ok). + // credentials it must not return the stats payload. httpTest { endpoint "${context.config.feHttpAddress}" uri "/rest/v2/manager/query/qerror/no_such_query_id" From 1aca54b25b128186a7fe1dbaf3de59450f59c6bc Mon Sep 17 00:00:00 2001 From: guoqiang Date: Wed, 1 Jul 2026 16:18:31 +0800 Subject: [PATCH 4/4] [fix](auth) adapt admin auth check for branch-4.0 backport of #65042 branch-4.0 does not have #60761 (HTTP API auth framework rework), so ActionAuthorizationInfo has no userIdentity field and checkAdminAuth() does not exist. Use the existing branch-4.0 pattern instead, matching setConfigBe/setConfigFe in the same file. --- .../apache/doris/httpv2/rest/manager/NodeAction.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/manager/NodeAction.java b/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/manager/NodeAction.java index 78f79057d35871..0525726eb96810 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/manager/NodeAction.java +++ b/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/manager/NodeAction.java @@ -613,8 +613,8 @@ public Object setConfigBe(HttpServletRequest request, HttpServletResponse respon @PostMapping("/{action}/be") public Object operateBackend(HttpServletRequest request, HttpServletResponse response, @PathVariable("action") String action, @RequestBody BackendReqInfo reqInfo) { - ActionAuthorizationInfo authInfo = executeCheckPassword(request, response); - checkAdminAuth(authInfo.userIdentity); + executeCheckPassword(request, response); + checkGlobalAuth(ConnectContext.get().getCurrentUserIdentity(), PrivPredicate.ADMIN); try { if (needRedirect(request.getScheme())) { return redirectToHttps(request); @@ -662,8 +662,8 @@ public Object operateBackend(HttpServletRequest request, HttpServletResponse res @PostMapping("/{action}/fe") public Object operateFrontends(HttpServletRequest request, HttpServletResponse response, @PathVariable("action") String action, @RequestBody FrontendReqInfo reqInfo) { - ActionAuthorizationInfo authInfo = executeCheckPassword(request, response); - checkAdminAuth(authInfo.userIdentity); + executeCheckPassword(request, response); + checkGlobalAuth(ConnectContext.get().getCurrentUserIdentity(), PrivPredicate.ADMIN); try { if (needRedirect(request.getScheme())) { return redirectToHttps(request); @@ -696,8 +696,8 @@ public Object operateFrontends(HttpServletRequest request, HttpServletResponse r @PostMapping("/{action}/broker") public Object operateBroker(HttpServletRequest request, HttpServletResponse response, @PathVariable("action") String action, @RequestBody BrokerReqInfo reqInfo) { - ActionAuthorizationInfo authInfo = executeCheckPassword(request, response); - checkAdminAuth(authInfo.userIdentity); + executeCheckPassword(request, response); + checkGlobalAuth(ConnectContext.get().getCurrentUserIdentity(), PrivPredicate.ADMIN); try { if (!Env.getCurrentEnv().isMaster()) { return redirectToMasterOrException(request, response);