Include user-defined attributes when building PolarisPrincipal from PrincipalEntity

CHANGELOG.md

@@ -139,6 +139,7 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti
 - Changed deprecated APIs in JUnit 5. This change will force downstream projects that pull in the Polaris test packages to adopt JUnit 6.
 - Added client id collision check during reset.
 - The ExternalCatalogFactory interface has been renamed to FederatedCatalogFactory. Its createCatalog() and createGenericCatalog() method signatures have been extended to include a `catalogProperties` parameter of type `Map<String, String>` for passing through proxy and timeout settings to federated catalog HTTP clients.
+- `PolarisPrincipal` construction from a `PrincipalEntity` now exposes the principal's user-defined properties (alongside internal properties) so external authorizers such as OPA and Ranger can use them for policy evaluation. Internal properties win on key collision so that system-managed values such as `client_id` cannot be shadowed by user input.
 
 ### Deprecations
 - The configuration option `polaris.event-listener.type` is deprecated and will be removed later. Please use `polaris.event-listener.types` instead.

polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisPrincipal.java

@@ -19,6 +19,7 @@
 package org.apache.polaris.core.auth;
 
 import java.security.Principal;
+import java.util.HashMap;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
@@ -34,35 +35,45 @@ public interface PolarisPrincipal extends Principal {
    * Creates a new instance of {@link PolarisPrincipal} from the given {@link PrincipalEntity} and
    * roles.
    *
-   * <p>The created principal will have the same ID and name as the {@link PrincipalEntity}, and its
-   * properties will be derived from the internal properties of the entity.
+   * <p>The created principal will have the same name as the {@link PrincipalEntity}, and its
+   * properties will be the merger of the entity's user-defined properties and its internal
+   * properties. If a key appears in both maps, the internal-property value wins, so that
+   * system-managed values (for example the {@code client_id}) cannot be shadowed by user input.
    *
    * @param principalEntity the principal entity representing the user or service
    * @param roles the set of roles associated with the principal
    */
   static PolarisPrincipal of(PrincipalEntity principalEntity, Set<String> roles) {
-    return of(
-        principalEntity.getName(),
-        principalEntity.getInternalPropertiesAsMap(),
-        roles,
-        Optional.empty());
+    return of(principalEntity, roles, Optional.empty());
   }
 
   /**
    * Creates a new instance of {@link PolarisPrincipal} from the given {@link PrincipalEntity} and
    * roles.
    *
-   * <p>The created principal will have the same ID and name as the {@link PrincipalEntity}, and its
-   * properties will be derived from the internal properties of the entity.
+   * <p>The created principal will have the same name as the {@link PrincipalEntity}, and its
+   * properties will be the merger of the entity's user-defined properties and its internal
+   * properties. If a key appears in both maps, the internal-property value wins, so that
+   * system-managed values (for example the {@code client_id}) cannot be shadowed by user input.
    *
    * @param principalEntity the principal entity representing the user or service
    * @param roles the set of roles associated with the principal
    * @param token the access token of the current user
    */
   static PolarisPrincipal of(
       PrincipalEntity principalEntity, Set<String> roles, Optional<String> token) {
-    return of(
-        principalEntity.getName(), principalEntity.getInternalPropertiesAsMap(), roles, token);
+    return of(principalEntity.getName(), mergeEntityProperties(principalEntity), roles, token);
+  }
+
+  /**
+   * Merges the user-defined and internal property maps of a {@link PrincipalEntity}. On key
+   * collision the internal value wins, so that system-managed properties (for example {@code
+   * client_id}) cannot be shadowed by user-supplied properties.
+   */
+  private static Map<String, String> mergeEntityProperties(PrincipalEntity principalEntity) {
+    Map<String, String> merged = new HashMap<>(principalEntity.getPropertiesAsMap());
+    merged.putAll(principalEntity.getInternalPropertiesAsMap());
+    return merged;
   }
 
   /**

polaris-core/src/test/java/org/apache/polaris/core/auth/PolarisPrincipalTest.java

@@ -0,0 +1,100 @@
+/*
+ * 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.polaris.core.auth;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+import org.apache.polaris.core.entity.PolarisEntityConstants;
+import org.apache.polaris.core.entity.PrincipalEntity;
+import org.junit.jupiter.api.Test;
+
+class PolarisPrincipalTest {
+
+  private static final String CLIENT_ID_KEY = PolarisEntityConstants.getClientIdPropertyName();
+
+  @Test
+  void ofPrincipalEntityExposesUserDefinedProperties() {
+    PrincipalEntity principalEntity =
+        new PrincipalEntity.Builder()
+            .setName("alice")
+            .setClientId("client-123")
+            .setProperties(Map.of("region", "northamerica", "department", "finance"))
+            .build();
+
+    PolarisPrincipal principal = PolarisPrincipal.of(principalEntity, Set.of("auditor"));
+
+    assertThat(principal.getName()).isEqualTo("alice");
+    assertThat(principal.getRoles()).containsExactly("auditor");
+    assertThat(principal.getProperties())
+        .containsEntry("region", "northamerica")
+        .containsEntry("department", "finance")
+        .containsEntry(CLIENT_ID_KEY, "client-123");
+  }
+
+  @Test
+  void ofPrincipalEntityWithTokenExposesUserDefinedProperties() {
+    PrincipalEntity principalEntity =
+        new PrincipalEntity.Builder()
+            .setName("bob")
+            .setClientId("client-456")
+            .setProperties(Map.of("team", "platform"))
+            .build();
+
+    PolarisPrincipal principal =
+        PolarisPrincipal.of(principalEntity, Set.of("admin"), Optional.of("access-token"));
+
+    assertThat(principal.getName()).isEqualTo("bob");
+    assertThat(principal.getRoles()).containsExactly("admin");
+    assertThat(principal.getToken()).contains("access-token");
+    assertThat(principal.getProperties())
+        .containsEntry("team", "platform")
+        .containsEntry(CLIENT_ID_KEY, "client-456");
+  }
+
+  @Test
+  void internalPropertiesTakePrecedenceOverUserDefinedOnCollision() {
+    // Regression guard: if anyone later flips the merge order so that user-defined values win,
+    // a caller could shadow a system-managed key such as client_id by setting a property of the
+    // same name on the principal entity. This test pins the safe direction.
+    PrincipalEntity principalEntity =
+        new PrincipalEntity.Builder()
+            .setName("eve")
+            .setClientId("legitimate-client-id")
+            .setProperties(Map.of(CLIENT_ID_KEY, "spoofed-client-id"))
+            .build();
+
+    PolarisPrincipal principal = PolarisPrincipal.of(principalEntity, Set.of());
+
+    // System-managed value wins; user-supplied value is discarded.
+    assertThat(principal.getProperties()).containsEntry(CLIENT_ID_KEY, "legitimate-client-id");
+  }
+
+  @Test
+  void ofPrincipalEntityWithoutUserPropertiesStillExposesInternalProperties() {
+    PrincipalEntity principalEntity =
+        new PrincipalEntity.Builder().setName("svc").setClientId("client-789").build();
+
+    PolarisPrincipal principal = PolarisPrincipal.of(principalEntity, Set.of());
+
+    assertThat(principal.getProperties()).containsEntry(CLIENT_ID_KEY, "client-789");
+  }
+}

runtime/service/src/test/java/org/apache/polaris/service/auth/DefaultAuthenticatorTest.java

@@ -389,11 +389,44 @@ void testPrincipalIdTakesPrecedenceOverName() {
     assertPrincipal(result, principalEntity, PRINCIPAL_ROLE1, PRINCIPAL_ROLE2);
   }
 
+  @Test
+  void testUserDefinedPropertiesArePreservedOnAuthenticatedPrincipal() {
+    // Given: a principal created via the management API with user-defined properties
+    String name = "principal-with-attrs";
+    PrincipalEntity created =
+        createPrincipal(name, Map.of("region", "northamerica", "department", "finance"));
+    PolarisCredential credentials =
+        PolarisCredential.of(null, name, Set.of(DefaultAuthenticator.PRINCIPAL_ROLE_ALL));
+
+    // When: authenticating the principal
+    PolarisPrincipal result = authenticator.authenticate(credentials);
+
+    // Then: user-defined properties from the management API are exposed alongside
+    // system-managed internal properties (client_id) — this is what authorizers such
+    // as OPA and Ranger consume as user attributes for policy evaluation.
+    assertThat(result).isNotNull();
+    assertThat(result.getName()).isEqualTo(name);
+    assertThat(result.getProperties())
+        .containsEntry("region", "northamerica")
+        .containsEntry("department", "finance")
+        .containsKey(PolarisEntityConstants.getClientIdPropertyName());
+    // Sanity check the entity itself carried those properties.
+    assertThat(created.getPropertiesAsMap())
+        .containsEntry("region", "northamerica")
+        .containsEntry("department", "finance");
+  }
+
   private PrincipalEntity createPrincipal(String name, String... roles) {
+    return createPrincipal(name, Map.of(), roles);
+  }
+
+  private PrincipalEntity createPrincipal(
+      String name, Map<String, String> properties, String... roles) {
 
     PrincipalWithCredentialsCredentials credentials =
         newAdminService()
-            .createPrincipal(new PrincipalEntity.Builder().setName(name).build())
+            .createPrincipal(
+                new PrincipalEntity.Builder().setName(name).setProperties(properties).build())
             .getCredentials();
 
     metaStoreManager.rotatePrincipalSecrets(