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 c9e51f4842b303..1ad24aaf3d6010 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 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) { + ActionAuthorizationInfo authInfo = executeCheckPassword(request, response); + checkAdminAuth(authInfo.userIdentity); try { if (needRedirect(request.getScheme())) { return redirectToHttps(request); @@ -660,7 +662,9 @@ 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) { + ActionAuthorizationInfo authInfo = executeCheckPassword(request, response); + checkAdminAuth(authInfo.userIdentity); try { if (needRedirect(request.getScheme())) { return redirectToHttps(request); @@ -692,7 +696,9 @@ 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) { + 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 e619a81326e985..27b955522ae39e 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 @@ 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..5b1774a44d0193 --- /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}") + } +}