This is an automated email from the ASF dual-hosted git repository.

yuqi1129 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git


The following commit(s) were added to refs/heads/main by this push:
     new a27b8b33a7 [#12557] fix(lance): Preserve authenticated caller identity 
(#12563)
a27b8b33a7 is described below

commit a27b8b33a7cf3e8cbe0c7b7ab1cfe54e191d281c
Author: Qi Yu <[email protected]>
AuthorDate: Mon Aug 24 11:28:31 2026 +0800

    [#12557] fix(lance): Preserve authenticated caller identity (#12563)
    
    ### What changes were proposed in this pull request?
    
    - Preserve the authenticated caller identity and active roles for
    auxiliary-mode Lance REST requests.
    - Use the configured Lance service identity only when the request is
    anonymous.
    - Add unit tests covering authenticated callers, active roles, anonymous
    fallback, nested principals, and exception recovery.
    - Add integration tests verifying audit creators for both authenticated
    and anonymous requests.
    
    ### Why are the changes needed?
    
    The Lance service identity filter unconditionally executed requests as
    the configured service user. This replaced the principal already
    established by the authentication filter, causing authorization,
    ownership, and audit operations to use the service identity instead of
    the authenticated caller.
    
    Fix: #12557
    
    ### Does this PR introduce _any_ user-facing change?
    
    Yes. Authenticated auxiliary-mode Lance REST metadata operations now run
    as the authenticated caller. Anonymous requests continue to use the
    configured service identity.
    
    No API or configuration changes are introduced.
    
    ### How was this patch tested?
    
    - `./gradlew :lance:lance-rest-server:check -PskipITs`
    - `./gradlew :lance:lance-rest-server:test --tests
    org.apache.gravitino.lance.integration.test.LanceRESTServiceAuthIT
    -PskipDockerTests=false`
    - `git diff --check`
    
    All tests were run with proxy environment variables disabled.
---
 .../lance/service/LanceServiceIdentityFilter.java  | 20 +++++-
 .../integration/test/LanceRESTServiceAuthIT.java   | 57 +++++++++++++--
 .../service/TestLanceServiceIdentityFilter.java    | 80 ++++++++++++++++++++--
 3 files changed, 145 insertions(+), 12 deletions(-)

diff --git 
a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceServiceIdentityFilter.java
 
b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceServiceIdentityFilter.java
index 9f8430582d..2860dc4866 100644
--- 
a/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceServiceIdentityFilter.java
+++ 
b/lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceServiceIdentityFilter.java
@@ -26,9 +26,13 @@ import javax.servlet.ServletException;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
 import org.apache.gravitino.UserPrincipal;
+import org.apache.gravitino.auth.AuthConstants;
 import org.apache.gravitino.utils.PrincipalUtils;
 
-/** Executes Lance REST requests as the configured service identity in 
auxiliary mode. */
+/**
+ * Supplies the configured service identity for anonymous Lance REST requests 
in auxiliary mode.
+ * Authenticated requests keep their caller identity.
+ */
 public class LanceServiceIdentityFilter implements Filter {
 
   private final UserPrincipal servicePrincipal;
@@ -49,6 +53,18 @@ public class LanceServiceIdentityFilter implements Filter {
   public void doFilter(ServletRequest request, ServletResponse response, 
FilterChain chain)
       throws IOException, ServletException {
     try {
+      // AuthenticationFilter runs before this filter and keeps the rest of 
the filter chain inside
+      // the caller's Subject. Starting another doAs block here would make the 
service user the
+      // effective identity, so downstream code would no longer see the real 
caller. Continue the
+      // existing chain directly to preserve the caller's active roles and 
other principal details.
+      if 
(!AuthConstants.ANONYMOUS_USER.equals(PrincipalUtils.getCurrentUserName())) {
+        chain.doFilter(request, response);
+        return;
+      }
+
+      // When authentication is disabled, or simple authentication accepted an 
anonymous request,
+      // there is no user identity for internal Gravitino dispatcher calls. 
Use the configured
+      // service identity only for this fallback case.
       PrincipalUtils.doAs(
           servicePrincipal,
           () -> {
@@ -58,7 +74,7 @@ public class LanceServiceIdentityFilter implements Filter {
     } catch (IOException | ServletException e) {
       throw e;
     } catch (Exception e) {
-      throw new ServletException("Failed to execute as the Lance REST service 
identity", e);
+      throw new ServletException("Failed to execute the Lance REST request", 
e);
     }
   }
 
diff --git 
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceRESTServiceAuthIT.java
 
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceRESTServiceAuthIT.java
index f34fef6589..c21ca27f5a 100644
--- 
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceRESTServiceAuthIT.java
+++ 
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceRESTServiceAuthIT.java
@@ -19,6 +19,12 @@
  */
 package org.apache.gravitino.lance.integration.test;
 
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpResponse;
+import java.nio.charset.StandardCharsets;
+import java.util.Base64;
 import java.util.HashMap;
 import java.util.Map;
 import org.apache.arrow.memory.BufferAllocator;
@@ -28,6 +34,7 @@ import org.apache.gravitino.auth.AuthConstants;
 import org.apache.gravitino.client.GravitinoMetalake;
 import org.apache.gravitino.integration.test.util.BaseIT;
 import org.apache.gravitino.integration.test.util.GravitinoITUtils;
+import org.apache.gravitino.server.web.ObjectMapperProvider;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeAll;
@@ -36,16 +43,16 @@ import org.lance.namespace.LanceNamespace;
 import org.lance.namespace.model.CreateNamespaceRequest;
 
 /**
- * Verifies that the Lance REST service authenticates to the Gravitino server 
as its configured
- * identity rather than anonymously.
+ * Verifies the identity used by auxiliary-mode Lance REST metadata operations.
  *
- * <p>Before the service was given an {@code AuthDataProvider}, its requests 
to the Gravitino server
- * carried no authorization header and were recorded against the anonymous 
user. Objects created
- * through the Lance REST service therefore had {@code anonymous} as their 
creator.
+ * <p>An authenticated request must run as its caller so authorization, 
ownership, and audit data
+ * use the real user. A request without a user falls back to the configured 
service identity so
+ * internal Gravitino calls never run anonymously.
  */
 public class LanceRESTServiceAuthIT extends BaseIT {
 
   private static final String SIMPLE_USER_NAME = "lance_rest_service_user";
+  private static final String REQUEST_USER_NAME = "lance_rest_request_user";
   private static final String USER_NAME_CONFIG_KEY =
       "gravitino.lance-rest.gravitino-simple.user-name";
 
@@ -99,7 +106,7 @@ public class LanceRESTServiceAuthIT extends BaseIT {
   }
 
   @Test
-  public void testCatalogCreatedViaLanceRestIsNotAnonymous() {
+  public void testAnonymousRequestUsesConfiguredServiceIdentity() {
     String catalogName = GravitinoITUtils.genRandomName("lance_auth_catalog");
 
     CreateNamespaceRequest createNamespaceReq = new CreateNamespaceRequest();
@@ -117,6 +124,44 @@ public class LanceRESTServiceAuthIT extends BaseIT {
     metalake.dropCatalog(catalogName, true);
   }
 
+  @Test
+  public void testCatalogCreatedViaLanceRestUsesAuthenticatedCaller() throws 
Exception {
+    String catalogName = 
GravitinoITUtils.genRandomName("lance_auth_caller_catalog");
+    CreateNamespaceRequest createNamespaceReq = new CreateNamespaceRequest();
+    createNamespaceReq.addIdItem(catalogName);
+
+    String authHeader =
+        AuthConstants.AUTHORIZATION_BASIC_HEADER
+            + Base64.getEncoder()
+                .encodeToString((REQUEST_USER_NAME + 
":dummy").getBytes(StandardCharsets.UTF_8));
+    HttpRequest request =
+        HttpRequest.newBuilder()
+            .uri(
+                URI.create(
+                    getLanceRestServiceUrl()
+                        + "/v1/namespace/"
+                        + catalogName
+                        + "/create?delimiter=."))
+            .header(AuthConstants.HTTP_HEADER_AUTHORIZATION, authHeader)
+            .header(AuthConstants.X_GRAVITINO_ACTIVE_ROLES_HEADER, "NONE")
+            .header("Content-Type", "application/json")
+            .POST(
+                HttpRequest.BodyPublishers.ofString(
+                    
ObjectMapperProvider.objectMapper().writeValueAsString(createNamespaceReq)))
+            .build();
+    HttpResponse<String> response =
+        HttpClient.newHttpClient().send(request, 
HttpResponse.BodyHandlers.ofString());
+
+    Assertions.assertEquals(200, response.statusCode(), "Unexpected body: " + 
response.body());
+    Catalog catalog = metalake.loadCatalog(catalogName);
+    Assertions.assertEquals(
+        REQUEST_USER_NAME,
+        catalog.auditInfo().creator(),
+        "An authenticated Lance REST request should not be replaced by the 
service identity");
+
+    metalake.dropCatalog(catalogName, true);
+  }
+
   private String getLanceRestServiceUrl() {
     return String.format("http://%s:%d/lance";, "localhost", 
getLanceRESTServerPort());
   }
diff --git 
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/TestLanceServiceIdentityFilter.java
 
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/TestLanceServiceIdentityFilter.java
index 774056e91c..d0d942ce30 100644
--- 
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/TestLanceServiceIdentityFilter.java
+++ 
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/TestLanceServiceIdentityFilter.java
@@ -20,11 +20,16 @@ package org.apache.gravitino.lance.service;
 
 import static org.mockito.Mockito.mock;
 
+import java.io.IOException;
+import java.security.Principal;
+import java.util.List;
 import java.util.concurrent.atomic.AtomicReference;
 import javax.servlet.FilterChain;
+import javax.servlet.ServletException;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
 import org.apache.gravitino.UserPrincipal;
+import org.apache.gravitino.auth.ActiveRoles;
 import org.apache.gravitino.auth.AuthConstants;
 import org.apache.gravitino.utils.PrincipalUtils;
 import org.junit.jupiter.api.Assertions;
@@ -33,7 +38,34 @@ import org.junit.jupiter.api.Test;
 public class TestLanceServiceIdentityFilter {
 
   @Test
-  public void testBindsConfiguredServiceIdentity() throws Exception {
+  public void testPreservesAuthenticatedCallerAndActiveRoles() throws 
Exception {
+    LanceServiceIdentityFilter filter = new 
LanceServiceIdentityFilter("lance_rest_service_user");
+    ServletRequest request = mock(ServletRequest.class);
+    ServletResponse response = mock(ServletResponse.class);
+    ActiveRoles activeRoles = ActiveRoles.of(List.of("analyst"));
+    UserPrincipal caller = new 
UserPrincipal("request_user").withActiveRoles(activeRoles);
+    AtomicReference<Principal> principalInChain = new AtomicReference<>();
+    FilterChain chain =
+        (servletRequest, servletResponse) ->
+            principalInChain.set(PrincipalUtils.getCurrentPrincipal());
+
+    Assertions.assertEquals(AuthConstants.ANONYMOUS_USER, 
PrincipalUtils.getCurrentUserName());
+
+    PrincipalUtils.doAs(
+        caller,
+        () -> {
+          filter.doFilter(request, response, chain);
+          Assertions.assertSame(caller, PrincipalUtils.getCurrentPrincipal());
+          return null;
+        });
+
+    Assertions.assertSame(caller, principalInChain.get());
+    Assertions.assertEquals(activeRoles, ((UserPrincipal) 
principalInChain.get()).getActiveRoles());
+    Assertions.assertEquals(AuthConstants.ANONYMOUS_USER, 
PrincipalUtils.getCurrentUserName());
+  }
+
+  @Test
+  public void testBindsConfiguredServiceIdentityForAnonymousRequest() throws 
Exception {
     String userName = "lance_rest_service_user";
     LanceServiceIdentityFilter filter = new 
LanceServiceIdentityFilter(userName);
     ServletRequest request = mock(ServletRequest.class);
@@ -42,16 +74,56 @@ public class TestLanceServiceIdentityFilter {
     FilterChain chain =
         (servletRequest, servletResponse) -> 
userInChain.set(PrincipalUtils.getCurrentUserName());
 
+    filter.doFilter(request, response, chain);
+
+    Assertions.assertEquals(userName, userInChain.get());
     Assertions.assertEquals(AuthConstants.ANONYMOUS_USER, 
PrincipalUtils.getCurrentUserName());
+  }
+
+  @Test
+  public void testRestoresAnonymousIdentityAfterCheckedException() {
+    LanceServiceIdentityFilter filter = new 
LanceServiceIdentityFilter("lance_rest_service_user");
+    ServletRequest request = mock(ServletRequest.class);
+    ServletResponse response = mock(ServletResponse.class);
+    IOException failure = new IOException("expected failure");
+    FilterChain chain =
+        (servletRequest, servletResponse) -> {
+          Assertions.assertEquals("lance_rest_service_user", 
PrincipalUtils.getCurrentUserName());
+          throw failure;
+        };
+
+    IOException thrown =
+        Assertions.assertThrows(IOException.class, () -> 
filter.doFilter(request, response, chain));
+
+    Assertions.assertSame(failure, thrown);
+    Assertions.assertEquals(AuthConstants.ANONYMOUS_USER, 
PrincipalUtils.getCurrentUserName());
+  }
+
+  @Test
+  public void testPreservesAuthenticatedCallerAfterUnexpectedException() 
throws Exception {
+    LanceServiceIdentityFilter filter = new 
LanceServiceIdentityFilter("lance_rest_service_user");
+    ServletRequest request = mock(ServletRequest.class);
+    ServletResponse response = mock(ServletResponse.class);
+    UserPrincipal outerCaller = new UserPrincipal("outer_user");
+    RuntimeException failure = new RuntimeException("expected failure");
+    FilterChain chain =
+        (servletRequest, servletResponse) -> {
+          Assertions.assertSame(outerCaller, 
PrincipalUtils.getCurrentPrincipal());
+          throw failure;
+        };
 
     PrincipalUtils.doAs(
-        new UserPrincipal("request_user"),
+        outerCaller,
         () -> {
-          filter.doFilter(request, response, chain);
+          ServletException thrown =
+              Assertions.assertThrows(
+                  ServletException.class, () -> filter.doFilter(request, 
response, chain));
+          Assertions.assertSame(failure, thrown.getCause());
+          Assertions.assertEquals("Failed to execute the Lance REST request", 
thrown.getMessage());
+          Assertions.assertSame(outerCaller, 
PrincipalUtils.getCurrentPrincipal());
           return null;
         });
 
-    Assertions.assertEquals(userName, userInChain.get());
     Assertions.assertEquals(AuthConstants.ANONYMOUS_USER, 
PrincipalUtils.getCurrentUserName());
   }
 }

Reply via email to