This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.1
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.1 by this push:
new bdb906e006f branch-4.1: [fix](auth) add auth check for manager node
and query qerror REST APIs (#65080)
bdb906e006f is described below
commit bdb906e006fbdc990e7d827d7e8b2fa4026c804a
Author: Calvin Kirs <[email protected]>
AuthorDate: Wed Jul 1 17:47:32 2026 +0800
branch-4.1: [fix](auth) add auth check for manager node and query qerror
REST APIs (#65080)
cherry-pick apache/doris#65042
cherry-pick apache/doris#59708
---------
Co-authored-by: heguanhui <[email protected]>
---
.../doris/httpv2/rest/manager/NodeAction.java | 14 ++-
.../httpv2/rest/manager/QueryProfileAction.java | 10 +-
.../auth_p0/test_http_node_action_auth.groovy | 113 +++++++++++++++++++++
3 files changed, 132 insertions(+), 5 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 c9e51f4842b..1ad24aaf3d6 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
@@ -612,8 +612,10 @@ public class NodeAction extends RestBaseController {
}
@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) {
+ ActionAuthorizationInfo authInfo = executeCheckPassword(request,
response);
+ checkAdminAuth(authInfo.userIdentity);
try {
if (needRedirect(request.getScheme())) {
return redirectToHttps(request);
@@ -660,7 +662,9 @@ public class NodeAction extends RestBaseController {
@PostMapping("/{action}/fe")
public Object operateFrontends(HttpServletRequest request,
HttpServletResponse response,
- @PathVariable String action, @RequestBody FrontendReqInfo reqInfo)
{
+ @PathVariable("action") String action, @RequestBody
FrontendReqInfo reqInfo) {
+ ActionAuthorizationInfo authInfo = executeCheckPassword(request,
response);
+ checkAdminAuth(authInfo.userIdentity);
try {
if (needRedirect(request.getScheme())) {
return redirectToHttps(request);
@@ -692,7 +696,9 @@ public class NodeAction extends RestBaseController {
@PostMapping("/{action}/broker")
public Object operateBroker(HttpServletRequest request,
HttpServletResponse response,
- @PathVariable String action, @RequestBody
BrokerReqInfo reqInfo) {
+ @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 e619a81326e..27b955522ae 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
@@ -373,7 +373,15 @@ public class QueryProfileAction extends RestBaseController
{
* Query qError.
*/
@RequestMapping(path = "/qerror/{id}", method = RequestMethod.GET)
- public ResponseEntity<String> 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 00000000000..5b1774a44d0
--- /dev/null
+++ b/regression-test/suites/auth_p0/test_http_node_action_auth.groovy
@@ -0,0 +1,113 @@
+// 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.
+//
+// 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 = { user_name, password, action, check_func ->
+ httpTest {
+ basicAuthorization "${user_name}", "${password}"
+ endpoint "${context.config.feHttpAddress}"
+ uri "/rest/v2/manager/node/${action}/fe"
+ op "post"
+ body """{"role": "OBSERVER", "hostPort": "${bogusFe}"}"""
+ check check_func
+ }
+ }
+
+ def operateBe = { user_name, password, action, check_func ->
+ httpTest {
+ basicAuthorization "${user_name}", "${password}"
+ endpoint "${context.config.feHttpAddress}"
+ uri "/rest/v2/manager/node/${action}/be"
+ op "post"
+ body """{"hostPorts": ["${bogusBe}"]}"""
+ check check_func
+ }
+ }
+
+ // A non-admin user must be rejected by the ADMIN privilege check.
+ // 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(user, pwd, "ADD") {
+ 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. 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("drop fe (admin) body:${body}")
+ assertFalse("${body}".contains("Unauthorized"))
+ }
+
+ operateBe.call(user, pwd, "DROP") {
+ respCode, 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.
+ 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}")
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
