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

roryqi 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 4d33878492 [#9213]: fix(iceberg): Override namespace owner with OAuth 
authenticated user (#9212)
4d33878492 is described below

commit 4d338784921dbabcc49b149251834c810d726700
Author: Bharath Krishna <[email protected]>
AuthorDate: Fri Nov 21 21:52:25 2025 -0800

    [#9213]: fix(iceberg): Override namespace owner with OAuth authenticated 
user (#9212)
    
    ### What changes were proposed in this pull request?
    Modified IcebergNamespaceOperationExecutor to override the
    client-provided owner property with the authenticated user from
    OAuth/JWT tokens when creating Iceberg schemas.
    
    ### Why are the changes needed?
    When namespaces are created via Spark or other Iceberg clients, the
    client sends its own 'owner' property value (e.g., 'spark', 'system',
    etc.) in the CreateNamespaceRequest. This results in incorrect namespace
    ownership being stored in the metadata, making audit trails unreliable
    and not reflecting the actual authenticated user.
    
    The fix ensures that schema ownership reflects the actual authenticated
    user rather than the client's environment or default values.
    
    Fix: #9213
    
    ### Does this PR introduce _any_ user-facing change?
    Yes. Namespaces created through the Iceberg REST API will now show the
    authenticated user (from OAuth/JWT token) as the owner instead of the
    value sent by the client (e.g., 'spark')
    
    ### How was this patch tested?
    
    Unit tests
---
 .../IcebergNamespaceOperationExecutor.java         |  31 +++++
 .../TestIcebergNamespaceOperationExecutor.java     | 137 +++++++++++++++++++++
 2 files changed, 168 insertions(+)

diff --git 
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergNamespaceOperationExecutor.java
 
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergNamespaceOperationExecutor.java
index e584d43202..35bd179292 100644
--- 
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergNamespaceOperationExecutor.java
+++ 
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/dispatcher/IcebergNamespaceOperationExecutor.java
@@ -19,6 +19,10 @@
 
 package org.apache.gravitino.iceberg.service.dispatcher;
 
+import java.util.HashMap;
+import java.util.Map;
+import org.apache.gravitino.auth.AuthConstants;
+import org.apache.gravitino.catalog.lakehouse.iceberg.IcebergConstants;
 import org.apache.gravitino.iceberg.service.IcebergCatalogWrapperManager;
 import org.apache.gravitino.listener.api.event.IcebergRequestContext;
 import org.apache.iceberg.catalog.Namespace;
@@ -30,9 +34,14 @@ import 
org.apache.iceberg.rest.responses.GetNamespaceResponse;
 import org.apache.iceberg.rest.responses.ListNamespacesResponse;
 import org.apache.iceberg.rest.responses.LoadTableResponse;
 import org.apache.iceberg.rest.responses.UpdateNamespacePropertiesResponse;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class IcebergNamespaceOperationExecutor implements 
IcebergNamespaceOperationDispatcher {
 
+  private static final Logger LOG =
+      LoggerFactory.getLogger(IcebergNamespaceOperationExecutor.class);
+
   private IcebergCatalogWrapperManager icebergCatalogWrapperManager;
 
   public IcebergNamespaceOperationExecutor(
@@ -43,6 +52,28 @@ public class IcebergNamespaceOperationExecutor implements 
IcebergNamespaceOperat
   @Override
   public CreateNamespaceResponse createNamespace(
       IcebergRequestContext context, CreateNamespaceRequest 
createNamespaceRequest) {
+    String authenticatedUser = context.userName();
+    if (!AuthConstants.ANONYMOUS_USER.equals(authenticatedUser)) {
+      String existingOwner = 
createNamespaceRequest.properties().get(IcebergConstants.OWNER);
+
+      // Override the owner as the authenticated user if different from 
authenticated user
+      if (!authenticatedUser.equals(existingOwner)) {
+        Map<String, String> properties = new 
HashMap<>(createNamespaceRequest.properties());
+        properties.put(IcebergConstants.OWNER, authenticatedUser);
+        LOG.debug(
+            "Overriding namespace owner from '{}' to authenticated user: '{}'",
+            existingOwner,
+            authenticatedUser);
+
+        // CreateNamespaceRequest is immutable, so we need to rebuild it with 
modified properties
+        createNamespaceRequest =
+            CreateNamespaceRequest.builder()
+                .withNamespace(createNamespaceRequest.namespace())
+                .setProperties(properties)
+                .build();
+      }
+    }
+
     return icebergCatalogWrapperManager
         .getCatalogWrapper(context.catalogName())
         .createNamespace(createNamespaceRequest);
diff --git 
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/dispatcher/TestIcebergNamespaceOperationExecutor.java
 
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/dispatcher/TestIcebergNamespaceOperationExecutor.java
new file mode 100644
index 0000000000..89f5b3929e
--- /dev/null
+++ 
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/dispatcher/TestIcebergNamespaceOperationExecutor.java
@@ -0,0 +1,137 @@
+/*
+ * 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.gravitino.iceberg.service.dispatcher;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.HashMap;
+import java.util.Map;
+import org.apache.gravitino.catalog.lakehouse.iceberg.IcebergConstants;
+import org.apache.gravitino.iceberg.service.CatalogWrapperForREST;
+import org.apache.gravitino.iceberg.service.IcebergCatalogWrapperManager;
+import org.apache.gravitino.listener.api.event.IcebergRequestContext;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.rest.requests.CreateNamespaceRequest;
+import org.apache.iceberg.rest.responses.CreateNamespaceResponse;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentCaptor;
+
+public class TestIcebergNamespaceOperationExecutor {
+
+  private IcebergNamespaceOperationExecutor executor;
+  private IcebergCatalogWrapperManager mockWrapperManager;
+  private CatalogWrapperForREST mockCatalogWrapper;
+  private IcebergRequestContext mockContext;
+
+  @BeforeEach
+  public void setUp() {
+    mockWrapperManager = mock(IcebergCatalogWrapperManager.class);
+    mockCatalogWrapper = mock(CatalogWrapperForREST.class);
+    executor = new IcebergNamespaceOperationExecutor(mockWrapperManager);
+
+    mockContext = mock(IcebergRequestContext.class);
+    when(mockContext.catalogName()).thenReturn("test_catalog");
+    
when(mockWrapperManager.getCatalogWrapper("test_catalog")).thenReturn(mockCatalogWrapper);
+  }
+
+  @Test
+  public void testCreateNamespaceOverridesOwnerWithAuthenticatedUser() {
+    String authenticatedUser = "[email protected]";
+    String clientProvidedOwner = "spark";
+
+    Map<String, String> properties = new HashMap<>();
+    properties.put(IcebergConstants.OWNER, clientProvidedOwner);
+
+    CreateNamespaceRequest originalRequest =
+        CreateNamespaceRequest.builder()
+            .withNamespace(Namespace.of("test_namespace"))
+            .setProperties(properties)
+            .build();
+
+    when(mockContext.userName()).thenReturn(authenticatedUser);
+    CreateNamespaceResponse mockResponse = mock(CreateNamespaceResponse.class);
+    when(mockCatalogWrapper.createNamespace(any())).thenReturn(mockResponse);
+
+    executor.createNamespace(mockContext, originalRequest);
+
+    ArgumentCaptor<CreateNamespaceRequest> requestCaptor =
+        ArgumentCaptor.forClass(CreateNamespaceRequest.class);
+    verify(mockCatalogWrapper).createNamespace(requestCaptor.capture());
+
+    CreateNamespaceRequest capturedRequest = requestCaptor.getValue();
+    String actualOwner = 
capturedRequest.properties().get(IcebergConstants.OWNER);
+
+    Assertions.assertEquals(authenticatedUser, actualOwner);
+    Assertions.assertNotEquals(clientProvidedOwner, actualOwner);
+  }
+
+  @Test
+  public void testCreateNamespaceAddsOwnerWhenMissing() {
+    String authenticatedUser = "[email protected]";
+
+    CreateNamespaceRequest originalRequest =
+        
CreateNamespaceRequest.builder().withNamespace(Namespace.of("test_namespace")).build();
+
+    when(mockContext.userName()).thenReturn(authenticatedUser);
+    CreateNamespaceResponse mockResponse = mock(CreateNamespaceResponse.class);
+    when(mockCatalogWrapper.createNamespace(any())).thenReturn(mockResponse);
+
+    executor.createNamespace(mockContext, originalRequest);
+
+    ArgumentCaptor<CreateNamespaceRequest> requestCaptor =
+        ArgumentCaptor.forClass(CreateNamespaceRequest.class);
+    verify(mockCatalogWrapper).createNamespace(requestCaptor.capture());
+
+    String actualOwner = 
requestCaptor.getValue().properties().get(IcebergConstants.OWNER);
+    Assertions.assertEquals(authenticatedUser, actualOwner);
+  }
+
+  @Test
+  public void testCreateNamespacePreservesOwnerForAnonymousUser() {
+    String clientProvidedOwner = "spark";
+
+    Map<String, String> properties = new HashMap<>();
+    properties.put(IcebergConstants.OWNER, clientProvidedOwner);
+
+    CreateNamespaceRequest originalRequest =
+        CreateNamespaceRequest.builder()
+            .withNamespace(Namespace.of("test_namespace"))
+            .setProperties(properties)
+            .build();
+
+    when(mockContext.userName()).thenReturn("anonymous");
+    CreateNamespaceResponse mockResponse = mock(CreateNamespaceResponse.class);
+    when(mockCatalogWrapper.createNamespace(any())).thenReturn(mockResponse);
+
+    executor.createNamespace(mockContext, originalRequest);
+
+    ArgumentCaptor<CreateNamespaceRequest> requestCaptor =
+        ArgumentCaptor.forClass(CreateNamespaceRequest.class);
+    verify(mockCatalogWrapper).createNamespace(requestCaptor.capture());
+
+    String actualOwner = 
requestCaptor.getValue().properties().get(IcebergConstants.OWNER);
+    Assertions.assertEquals(clientProvidedOwner, actualOwner);
+  }
+}

Reply via email to