This is an automated email from the ASF dual-hosted git repository. yiguolei pushed a commit to branch branch-4.2 in repository https://gitbox.apache.org/repos/asf/doris.git
commit 8cbfea08b3e80e6b2886a6582d3a37b81f86cbd5 Author: Calvin Kirs <[email protected]> AuthorDate: Tue Sep 29 21:46:39 2026 +0800 branch-4.1: [fix](fe) Apply the same checks to HTTP Basic and cookie requests on /rest/v1 (#68635) https://github.com/apache/doris/pull/68549 --- .../doris/httpv2/controller/BaseController.java | 9 +- .../doris/httpv2/controller/LoginController.java | 11 +- .../controller/BaseControllerBasicAuthTest.java | 188 +++++++++++++++++++++ .../suites/auth_p0/test_http_rest_v1_auth.groovy | 90 ++++++++++ 4 files changed, 295 insertions(+), 3 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/httpv2/controller/BaseController.java b/fe/fe-core/src/main/java/org/apache/doris/httpv2/controller/BaseController.java index 5a2b97e44d8..7e09ea0eaf4 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/httpv2/controller/BaseController.java +++ b/fe/fe-core/src/main/java/org/apache/doris/httpv2/controller/BaseController.java @@ -77,8 +77,13 @@ public class BaseController { ActionAuthorizationInfo authInfo = getAuthorizationInfo(request); UserIdentity currentUser = checkPassword(authInfo, request); - if (Config.isCloudMode() && checkAuth) { - checkInstanceOverdue(currentUser); + // The cookie branch below requires ADMIN_OR_NODE whenever checkAuth is set, in every deployment + // mode, so this branch must as well: which of the two ways a caller authenticates must not change + // what it is allowed to reach. Only the overdue fence is specific to cloud mode. + if (checkAuth) { + if (Config.isCloudMode()) { + checkInstanceOverdue(currentUser); + } checkGlobalAuth(currentUser, PrivPredicate.ADMIN_OR_NODE); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/httpv2/controller/LoginController.java b/fe/fe-core/src/main/java/org/apache/doris/httpv2/controller/LoginController.java index fbf0a16b02b..9d7ad312596 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/httpv2/controller/LoginController.java +++ b/fe/fe-core/src/main/java/org/apache/doris/httpv2/controller/LoginController.java @@ -17,6 +17,9 @@ package org.apache.doris.httpv2.controller; +import org.apache.doris.common.Config; +import org.apache.doris.qe.ConnectContext; + import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import org.springframework.web.bind.annotation.RequestMapping; @@ -32,7 +35,13 @@ public class LoginController extends BaseController { @RequestMapping(path = "/login", method = RequestMethod.POST) public Object login(HttpServletRequest request, HttpServletResponse response) { - checkAuthWithCookie(request, response); + // Login only establishes who the caller is. What the account may reach is decided on each /rest/v1 request + // that follows, where the session issued here is checked for the required privilege; this lets the UI + // tell an account that lacks it apart from a failed sign-in. + checkWithCookie(request, response, false); + if (Config.isCloudMode()) { + checkInstanceOverdue(ConnectContext.get().getCurrentUserIdentity()); + } Map<String, Object> msg = new HashMap<>(); msg.put("code", 200); msg.put("msg", "Login success!"); diff --git a/fe/fe-core/src/test/java/org/apache/doris/httpv2/controller/BaseControllerBasicAuthTest.java b/fe/fe-core/src/test/java/org/apache/doris/httpv2/controller/BaseControllerBasicAuthTest.java new file mode 100644 index 00000000000..7febfcff270 --- /dev/null +++ b/fe/fe-core/src/test/java/org/apache/doris/httpv2/controller/BaseControllerBasicAuthTest.java @@ -0,0 +1,188 @@ +// 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. + +package org.apache.doris.httpv2.controller; + +import org.apache.doris.analysis.UserIdentity; +import org.apache.doris.common.Config; +import org.apache.doris.httpv2.HttpAuthManager.SessionValue; +import org.apache.doris.httpv2.controller.BaseController.ActionAuthorizationInfo; +import org.apache.doris.httpv2.exception.UnauthorizedException; +import org.apache.doris.httpv2.interceptor.AuthInterceptor; +import org.apache.doris.mysql.privilege.PrivPredicate; +import org.apache.doris.qe.ConnectContext; + +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.lang.reflect.Proxy; +import java.util.Map; + +/** + * HTTP Basic requests to the /rest/v1 surface, outside cloud mode. + */ +class BaseControllerBasicAuthTest { + private final UserIdentity analyst = UserIdentity.createAnalyzedUserIdentWithIp("analyst", "%"); + + private PrivPredicate checkedPredicate; + private SessionValue issuedSession; + + @BeforeEach + void setUp() { + Assertions.assertFalse(Config.isCloudMode()); + checkedPredicate = null; + issuedSession = null; + } + + @AfterEach + void tearDown() { + ConnectContext.remove(); + } + + @Test + void rejectsBasicAuthenticationWithoutPrivilege() { + AuthInterceptor interceptor = interceptor(false); + + Assertions.assertThrows(UnauthorizedException.class, + () -> interceptor.preHandle(request("/rest/v1/system"), response(), new Object())); + Assertions.assertEquals(PrivPredicate.ADMIN_OR_NODE, checkedPredicate); + Assertions.assertNull(issuedSession); + } + + @Test + void acceptsBasicAuthenticationWithPrivilege() { + AuthInterceptor interceptor = interceptor(true); + + Assertions.assertTrue(interceptor.preHandle(request("/rest/v1/system"), response(), new Object())); + Assertions.assertEquals(PrivPredicate.ADMIN_OR_NODE, checkedPredicate); + Assertions.assertEquals(analyst, issuedSession.currentUser); + } + + @Test + void loginAuthenticatesAnAccountWithoutPrivilege() { + // The UI tells an account that lacks the privilege apart from a failed sign-in, so login itself only + // authenticates; the privilege is checked on the requests that follow. + LoginController controller = new LoginController() { + @Override + public ActionAuthorizationInfo getAuthorizationInfo(HttpServletRequest request) { + return authorizationInfo(); + } + + @Override + protected UserIdentity checkPassword(ActionAuthorizationInfo authInfo, HttpServletRequest request) { + return analyst; + } + + @Override + protected void checkGlobalAuth(UserIdentity currentUser, PrivPredicate predicate) { + checkedPredicate = predicate; + throw new UnauthorizedException("Access denied"); + } + + @Override + protected void addSession(HttpServletRequest request, HttpServletResponse response, + SessionValue value) { + issuedSession = value; + } + }; + + @SuppressWarnings("unchecked") + Map<String, Object> result = (Map<String, Object>) controller.login(request("/rest/v1/login"), response()); + Assertions.assertEquals(200, result.get("code")); + Assertions.assertNull(checkedPredicate); + Assertions.assertEquals(analyst, issuedSession.currentUser); + } + + private AuthInterceptor interceptor(boolean privileged) { + return new AuthInterceptor() { + @Override + public ActionAuthorizationInfo getAuthorizationInfo(HttpServletRequest request) { + return authorizationInfo(); + } + + @Override + protected UserIdentity checkPassword(ActionAuthorizationInfo authInfo, HttpServletRequest request) { + return analyst; + } + + @Override + protected void checkGlobalAuth(UserIdentity currentUser, PrivPredicate predicate) { + checkedPredicate = predicate; + Assertions.assertEquals(analyst, currentUser); + if (!privileged) { + throw new UnauthorizedException("Access denied"); + } + } + + @Override + protected void addSession(HttpServletRequest request, HttpServletResponse response, + SessionValue value) { + issuedSession = value; + } + }; + } + + private ActionAuthorizationInfo authorizationInfo() { + ActionAuthorizationInfo authInfo = new ActionAuthorizationInfo(); + authInfo.fullUserName = analyst.getQualifiedUser(); + authInfo.password = "secret"; + authInfo.remoteIp = "127.0.0.1"; + return authInfo; + } + + private HttpServletRequest request(String requestUri) { + return (HttpServletRequest) Proxy.newProxyInstance( + HttpServletRequest.class.getClassLoader(), + new Class<?>[] {HttpServletRequest.class}, + (proxy, method, args) -> { + switch (method.getName()) { + case "getMethod": + return "GET"; + case "getRequestURI": + return requestUri; + case "getHeader": + return "Authorization".equals(args[0]) ? "Basic ignored-by-test" : null; + default: + return defaultValue(method.getReturnType()); + } + }); + } + + private HttpServletResponse response() { + return (HttpServletResponse) Proxy.newProxyInstance( + HttpServletResponse.class.getClassLoader(), + new Class<?>[] {HttpServletResponse.class}, + (proxy, method, args) -> defaultValue(method.getReturnType())); + } + + private Object defaultValue(Class<?> returnType) { + if (!returnType.isPrimitive()) { + return null; + } + if (returnType == boolean.class) { + return false; + } + if (returnType == char.class) { + return '\0'; + } + return 0; + } +} diff --git a/regression-test/suites/auth_p0/test_http_rest_v1_auth.groovy b/regression-test/suites/auth_p0/test_http_rest_v1_auth.groovy new file mode 100644 index 00000000000..f6cdd0444c8 --- /dev/null +++ b/regression-test/suites/auth_p0/test_http_rest_v1_auth.groovy @@ -0,0 +1,90 @@ +// 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. + +// The /rest/v1 endpoints behind the Web UI require ADMIN_OR_NODE. HTTP Basic requests are checked for it +// just as session-cookie requests are, in every deployment mode. /rest/v1/login itself only authenticates, +// so the UI can tell an account without the privilege apart from a failed sign-in. +suite("test_http_rest_v1_auth", "p0,auth") { + String user = "test_http_rest_v1_auth_user" + String pwd = 'C123_567p' + try_sql("DROP USER ${user}") + sql """CREATE USER '${user}' IDENTIFIED BY '${pwd}'""" + + def uris = [ + "/rest/v1/system?path=/", + "/rest/v1/config/fe", + "/rest/v1/session", + "/rest/v1/query_profile", + "/rest/v1/log", + "/rest/v1/ha" + ] + + def getRestV1 = { uriPath, checkFunc -> + httpTest { + basicAuthorization "${user}", "${pwd}" + endpoint "${context.config.feHttpAddress}" + uri uriPath + op "get" + check checkFunc + } + } + + def login = { checkFunc -> + httpTest { + basicAuthorization "${user}", "${pwd}" + endpoint "${context.config.feHttpAddress}" + uri "/rest/v1/login" + op "post" + body "{}" + check checkFunc + } + } + + uris.each { uriPath -> + getRestV1.call(uriPath) { + respCode, body -> + log.info("${uriPath} (no privilege) body:${body}") + assertEquals(200, respCode) + assertEquals(401, parseJson(body).code) + } + } + + login.call { + respCode, body -> + log.info("login (no privilege) body:${body}") + assertEquals(200, respCode) + assertEquals(200, parseJson(body).code) + } + + sql """GRANT 'admin' TO '${user}'""" + + uris.each { uriPath -> + getRestV1.call(uriPath) { + respCode, body -> + log.info("${uriPath} (admin) body:${body}") + assertEquals(200, respCode) + assertEquals(0, parseJson(body).code) + } + } + + login.call { + respCode, body -> + log.info("login (admin) body:${body}") + assertEquals(200, respCode) + assertEquals(200, parseJson(body).code) + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
