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 3737c679da5 branch-4.1: [fix](fe) Apply the same checks to HTTP Basic
and cookie requests on /rest/v1 (#68635)
3737c679da5 is described below
commit 3737c679da53833d61ea07a8e5501848068a4eaf
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]
