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);
+ }
+}