This is an automated email from the ASF dual-hosted git repository.
CalvinKirs pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 94a791e6644 [fix](auth) add auth check for manager node and query
qerror REST APIs (#65042)
94a791e6644 is described below
commit 94a791e664469a9b1261b1779759380163c3547e
Author: Calvin Kirs <[email protected]>
AuthorDate: Wed Jul 1 14:33:00 2026 +0800
[fix](auth) add auth check for manager node and query qerror REST APIs
(#65042)
## Proposed changes
Several manager REST APIs under `/rest/v2/manager` were missing
authentication and/or authorization. This PR closes those gaps.
### 1. Node management endpoints — missing auth + authz
`POST /rest/v2/manager/node/{action}/fe`, `/{action}/be`,
`/{action}/broker` (`operateFrontends` / `operateBackend` /
`operateBroker`) could add or drop FE / BE / Broker nodes **without any
authentication or authorization**. Any caller able to reach the FE HTTP
port could change cluster topology.
Added, consistent with the sibling `set_config/fe` and `set_config/be`
endpoints:
```java
ActionAuthorizationInfo authInfo = executeCheckPassword(request, response);
checkAdminAuth(authInfo.userIdentity);
```
### 2. `GET /rest/v2/manager/query/qerror/{id}` (`getStats`) — fully
unauthenticated
This endpoint had **neither authentication nor authorization**: its
method signature didn't even take
`HttpServletRequest`/`HttpServletResponse`, so it could not call
`executeCheckPassword`, and the global `AuthInterceptor` only covers
`/rest/v1/**`. As a result it was reachable anonymously **even with
`enable_all_http_auth=true`**, leaking per-query stats-error
information.
Aligned it with the `/profile` and `/trace_id` endpoints — authenticate,
then restrict non-admin users to their own queries:
```java
executeCheckPassword(request, response);
try {
checkAuthByUserAndQueryId(id);
} catch (AuthenticationException e) {
return ResponseEntityBuilder.badRequest(e.getMessage());
}
```
## Test
Added `regression-test/suites/auth_p0/test_http_node_action_auth.groovy`
(`p0,auth,nonConcurrent`):
- a non-admin user calling `ADD /fe` and `ADD /be` is rejected;
- after `grant 'admin'`, the request passes the auth check;
- an unauthenticated call to `/qerror/{id}` is rejected.
FE compiles cleanly (`build.sh --fe`).
---
.../doris/httpv2/rest/manager/NodeAction.java | 6 ++
.../httpv2/rest/manager/QueryProfileAction.java | 10 +-
.../auth_p0/test_http_node_action_auth.groovy | 113 +++++++++++++++++++++
3 files changed, 128 insertions(+), 1 deletion(-)
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 04a3eb2a1c2..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
@@ -614,6 +614,8 @@ public class NodeAction extends RestBaseController {
@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);
@@ -661,6 +663,8 @@ public class NodeAction extends RestBaseController {
@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);
@@ -693,6 +697,8 @@ public class NodeAction extends RestBaseController {
@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 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]
