From a245fbb1915f7185927081ea91139ae8ea561402 Mon Sep 17 00:00:00 2001 From: prithvi Date: Tue, 12 May 2026 02:25:52 +0530 Subject: [PATCH] Include user-defined attributes when building PolarisPrincipal from PrincipalEntity --- CHANGELOG.md | 1 + .../polaris/core/auth/PolarisPrincipal.java | 33 ++++-- .../core/auth/PolarisPrincipalTest.java | 100 ++++++++++++++++++ .../auth/DefaultAuthenticatorTest.java | 35 +++++- 4 files changed, 157 insertions(+), 12 deletions(-) create mode 100644 polaris-core/src/test/java/org/apache/polaris/core/auth/PolarisPrincipalTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 844576b217b..486314e01b5 100644 --- a/CHANGELOG.md +++ b/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` 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. diff --git a/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisPrincipal.java b/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisPrincipal.java index 07e36b8018a..d9873bae949 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisPrincipal.java +++ b/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,26 +35,26 @@ public interface PolarisPrincipal extends Principal { * Creates a new instance of {@link PolarisPrincipal} from the given {@link PrincipalEntity} and * roles. * - *

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. + *

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 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. * - *

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. + *

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 @@ -61,8 +62,18 @@ static PolarisPrincipal of(PrincipalEntity principalEntity, Set roles) { */ static PolarisPrincipal of( PrincipalEntity principalEntity, Set roles, Optional 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 mergeEntityProperties(PrincipalEntity principalEntity) { + Map merged = new HashMap<>(principalEntity.getPropertiesAsMap()); + merged.putAll(principalEntity.getInternalPropertiesAsMap()); + return merged; } /** diff --git a/polaris-core/src/test/java/org/apache/polaris/core/auth/PolarisPrincipalTest.java b/polaris-core/src/test/java/org/apache/polaris/core/auth/PolarisPrincipalTest.java new file mode 100644 index 00000000000..ba7c58f5438 --- /dev/null +++ b/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"); + } +} diff --git a/runtime/service/src/test/java/org/apache/polaris/service/auth/DefaultAuthenticatorTest.java b/runtime/service/src/test/java/org/apache/polaris/service/auth/DefaultAuthenticatorTest.java index 20c00e58e1b..91796dcb44d 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/auth/DefaultAuthenticatorTest.java +++ b/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 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(