diff --git a/CHANGELOG.md b/CHANGELOG.md index 1929e87fdc..e8c194f2dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -83,6 +83,10 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti root principal already exists. Existing credentials are never returned or altered. - Authorization failure messages (HTTP 403 / `ForbiddenException` from `PolarisAuthorizerImpl`) now log the specific missing privilege(s) and the entity each was checked against server-side (at `INFO` level), e.g. `missing TABLE_CREATE on NAMESPACE 'ns1'`. The client-facing 403 response remains a generic message to avoid leaking authorization metadata to untrusted clients. Operators can correlate client errors to server logs using the existing `X-Request-ID` header (present in default log MDC as `requestId`). - The field `clientSecret` of the Polaris management API type `ResetPrincipalRequest` is now using `format: password`. This does not change the wire format, but code generated from the OpenAPI may require downstream changes. +- OPA and Ranger authorizers now receive user-defined principal properties from the backing + `PrincipalEntity` (exposed via `PolarisPrincipal.PRINCIPAL_ENTITY_ATTRIBUTE_KEY`), alongside + internal properties. Internal properties win on key collision so system-managed values such as + `client_id` cannot be shadowed by user input. ### Deprecations - Deprecated `ALLOW_EXTERNAL_TABLE_LOCATION`. Use `ALLOW_EXTERNAL_METADATA_FILE_LOCATION` for external metadata file locations, including catalog config `polaris.config.allow.external.metadata.file.location`. diff --git a/extensions/auth/opa/opa-input-schema.json b/extensions/auth/opa/opa-input-schema.json index 0a5b22d18f..e6a4922510 100644 --- a/extensions/auth/opa/opa-input-schema.json +++ b/extensions/auth/opa/opa-input-schema.json @@ -16,6 +16,12 @@ "items" : { "type" : "string" } + }, + "attributes" : { + "type" : "object", + "additionalProperties" : { + "type" : "string" + } } } }, diff --git a/extensions/auth/opa/src/main/java/org/apache/polaris/extension/auth/opa/OpaPolarisAuthorizer.java b/extensions/auth/opa/src/main/java/org/apache/polaris/extension/auth/opa/OpaPolarisAuthorizer.java index 286b7665a6..d5025eec95 100644 --- a/extensions/auth/opa/src/main/java/org/apache/polaris/extension/auth/opa/OpaPolarisAuthorizer.java +++ b/extensions/auth/opa/src/main/java/org/apache/polaris/extension/auth/opa/OpaPolarisAuthorizer.java @@ -24,7 +24,10 @@ import java.io.IOException; import java.net.URI; import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.UUID; import org.apache.hc.client5.http.classic.methods.HttpPost; @@ -56,6 +59,7 @@ import org.apache.polaris.core.auth.SingleTargetAuthorizationIntent; import org.apache.polaris.core.auth.TargetlessAuthorizationIntent; import org.apache.polaris.core.entity.PolarisBaseEntity; +import org.apache.polaris.core.entity.PrincipalEntity; import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; import org.apache.polaris.core.persistence.ResolvedPolarisEntity; import org.apache.polaris.extension.auth.opa.model.ImmutableActor; @@ -345,9 +349,24 @@ private ImmutableActor buildActor(PolarisPrincipal principal) { return ImmutableActor.builder() .principal(principal.getName()) .addAllRoles(principal.getRoles()) + .putAllAttributes(extractPrincipalAttributes(principal)) .build(); } + private static Map extractPrincipalAttributes(PolarisPrincipal principal) { + return principal + .getAttribute(PolarisPrincipal.PRINCIPAL_ENTITY_ATTRIBUTE_KEY, PrincipalEntity.class) + .map( + entity -> { + Map attributes = new HashMap<>(entity.getPropertiesAsMap()); + // Internal properties win on collision so system-managed values cannot be shadowed + // by user-supplied properties. + attributes.putAll(entity.getInternalPropertiesAsMap()); + return attributes; + }) + .orElse(Collections.emptyMap()); + } + private ImmutableContext buildContext() { return ImmutableContext.builder() .requestId(requestId != null ? requestId : UUID.randomUUID().toString()) diff --git a/extensions/auth/opa/src/main/java/org/apache/polaris/extension/auth/opa/model/Actor.java b/extensions/auth/opa/src/main/java/org/apache/polaris/extension/auth/opa/model/Actor.java index 6da3c5355b..9e1be78289 100644 --- a/extensions/auth/opa/src/main/java/org/apache/polaris/extension/auth/opa/model/Actor.java +++ b/extensions/auth/opa/src/main/java/org/apache/polaris/extension/auth/opa/model/Actor.java @@ -23,6 +23,7 @@ import com.fasterxml.jackson.databind.annotation.JsonNaming; import com.fasterxml.jackson.databind.annotation.JsonSerialize; import java.util.List; +import java.util.Map; import org.apache.polaris.immutables.PolarisImmutable; /** @@ -40,4 +41,10 @@ public interface Actor { /** The list of roles associated with the principal. */ List roles(); + + /** + * Optional attributes associated with the principal, such as user-defined properties from the + * backing {@link org.apache.polaris.core.entity.PrincipalEntity}. + */ + Map attributes(); } diff --git a/extensions/auth/opa/src/test/java/org/apache/polaris/extension/auth/opa/OpaPolarisAuthorizerTest.java b/extensions/auth/opa/src/test/java/org/apache/polaris/extension/auth/opa/OpaPolarisAuthorizerTest.java index bab45c2751..1e1133f39b 100644 --- a/extensions/auth/opa/src/test/java/org/apache/polaris/extension/auth/opa/OpaPolarisAuthorizerTest.java +++ b/extensions/auth/opa/src/test/java/org/apache/polaris/extension/auth/opa/OpaPolarisAuthorizerTest.java @@ -63,6 +63,7 @@ import org.apache.polaris.core.entity.PolarisEntity; import org.apache.polaris.core.entity.PolarisEntityConstants; import org.apache.polaris.core.entity.PolarisEntityType; +import org.apache.polaris.core.entity.PrincipalEntity; import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; import org.apache.polaris.core.persistence.ResolvedPolarisEntity; import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifest; @@ -96,7 +97,7 @@ void serializesBasicOpaInput() throws Exception { "test-realm"); PolarisPrincipal principal = - PolarisPrincipal.of("eve", Map.of("department", "finance"), Set.of("auditor")); + principalWithUserAttributes("eve", Map.of("department", "finance"), Set.of("auditor")); Set entities = Set.of(); PolarisResolvedPathWrapper target = new PolarisResolvedPathWrapper(List.of()); @@ -125,6 +126,11 @@ void serializesBasicOpaInput() throws Exception { // Verify realm is included for tenant isolation assertThat(input.get("context").has("realm")).as("context should contain realm").isTrue(); assertThat(input.get("context").get("realm").asText()).isEqualTo("test-realm"); + + // Verify user-defined principal properties are forwarded as actor attributes + var actor = input.get("actor"); + assertThat(actor.has("attributes")).as("Actor should have 'attributes' field").isTrue(); + assertThat(actor.get("attributes").get("department").asText()).isEqualTo("finance"); } finally { server.stop(0); } @@ -1215,7 +1221,7 @@ void requestIdIsIncludedInContextWhenProvided() throws Exception { "test-realm"); PolarisPrincipal principal = - PolarisPrincipal.of("eve", Map.of("department", "finance"), Set.of("auditor")); + principalWithUserAttributes("eve", Map.of("department", "finance"), Set.of("auditor")); PolarisResolvedPathWrapper target = new PolarisResolvedPathWrapper(List.of()); PolarisResolvedPathWrapper secondary = new PolarisResolvedPathWrapper(List.of()); @@ -1256,7 +1262,7 @@ void requestIdFallsBackToRandomUuidWhenNull() throws Exception { "test-realm"); PolarisPrincipal principal = - PolarisPrincipal.of("eve", Map.of("department", "finance"), Set.of("auditor")); + principalWithUserAttributes("eve", Map.of("department", "finance"), Set.of("auditor")); PolarisResolvedPathWrapper target = new PolarisResolvedPathWrapper(List.of()); PolarisResolvedPathWrapper secondary = new PolarisResolvedPathWrapper(List.of()); @@ -1279,6 +1285,69 @@ void requestIdFallsBackToRandomUuidWhenNull() throws Exception { } } + @Test + void actorAttributesIncludeUserDefinedPropertiesAndInternalWinsOnCollision() throws Exception { + final String[] capturedRequestBody = new String[1]; + + HttpServer server = createServerWithRequestCapture(capturedRequestBody); + try { + URI policyUri = + URI.create( + "http://localhost:" + server.getAddress().getPort() + "/v1/data/polaris/allow"); + OpaPolarisAuthorizer authorizer = + new OpaPolarisAuthorizer( + policyUri, + HttpClients.createDefault(), + JsonMapper.builder().build(), + null, + null, + "test-realm"); + + PrincipalEntity entity = + new PrincipalEntity.Builder() + .setName("eve") + .setProperties( + Map.of( + "department", + "finance", + PolarisEntityConstants.getClientIdPropertyName(), + "user-client-id")) + .setClientId("internal-client-id") + .build(); + PolarisPrincipal principal = + PolarisPrincipal.of( + "eve", + Map.of(PolarisPrincipal.PRINCIPAL_ENTITY_ATTRIBUTE_KEY, entity), + Set.of("auditor")); + + PolarisResolvedPathWrapper target = new PolarisResolvedPathWrapper(List.of()); + PolarisResolvedPathWrapper secondary = new PolarisResolvedPathWrapper(List.of()); + + assertThatNoException() + .isThrownBy( + () -> + authorizer.authorizeOrThrow( + principal, + Set.of(), + PolarisAuthorizableOperation.LOAD_VIEW, + target, + secondary)); + + ObjectMapper mapper = JsonMapper.builder().build(); + JsonNode root = mapper.readTree(capturedRequestBody[0]); + var actor = root.path("input").path("actor"); + assertThat(actor.path("attributes").path("department").asText()).isEqualTo("finance"); + assertThat( + actor + .path("attributes") + .path(PolarisEntityConstants.getClientIdPropertyName()) + .asText()) + .isEqualTo("internal-client-id"); + } finally { + server.stop(0); + } + } + private AuthorizationRequest requestWithCatalogTarget(PolarisPrincipal principal) { return new AuthorizationRequest( principal, @@ -1373,4 +1442,14 @@ private void verifyAuthorizationHeader(ClassicHttpRequest capturedRequest, Strin .isFalse(); } } + + private static PolarisPrincipal principalWithUserAttributes( + String name, Map userAttributes, Set roles) { + return PolarisPrincipal.of( + name, + Map.of( + PolarisPrincipal.PRINCIPAL_ENTITY_ATTRIBUTE_KEY, + new PrincipalEntity.Builder().setName(name).setProperties(userAttributes).build()), + roles); + } } diff --git a/extensions/auth/ranger/src/main/java/org/apache/polaris/extension/auth/ranger/utils/RangerUtils.java b/extensions/auth/ranger/src/main/java/org/apache/polaris/extension/auth/ranger/utils/RangerUtils.java index 117e982d30..93e77a22a6 100644 --- a/extensions/auth/ranger/src/main/java/org/apache/polaris/extension/auth/ranger/utils/RangerUtils.java +++ b/extensions/auth/ranger/src/main/java/org/apache/polaris/extension/auth/ranger/utils/RangerUtils.java @@ -131,7 +131,14 @@ private static Map getUserAttributes(PolarisPrincipal principal) Map properties = principal .getAttribute(PolarisPrincipal.PRINCIPAL_ENTITY_ATTRIBUTE_KEY, PrincipalEntity.class) - .map(PrincipalEntity::getInternalPropertiesAsMap) + .map( + entity -> { + Map merged = new HashMap<>(entity.getPropertiesAsMap()); + // Internal properties win on collision so system-managed values cannot be + // shadowed by user-supplied properties. + merged.putAll(entity.getInternalPropertiesAsMap()); + return merged; + }) .orElse(Collections.emptyMap()); return properties.isEmpty() ? Collections.emptyMap() : new HashMap<>(properties); diff --git a/extensions/auth/ranger/src/test/java/org/apache/polaris/extension/auth/ranger/utils/RangerUtilsTest.java b/extensions/auth/ranger/src/test/java/org/apache/polaris/extension/auth/ranger/utils/RangerUtilsTest.java new file mode 100644 index 0000000000..b4f6b878e6 --- /dev/null +++ b/extensions/auth/ranger/src/test/java/org/apache/polaris/extension/auth/ranger/utils/RangerUtilsTest.java @@ -0,0 +1,77 @@ +/* + * 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.extension.auth.ranger.utils; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.Map; +import java.util.Set; +import org.apache.polaris.core.auth.PolarisPrincipal; +import org.apache.polaris.core.entity.PolarisEntityConstants; +import org.apache.polaris.core.entity.PrincipalEntity; +import org.apache.ranger.authz.model.RangerUserInfo; +import org.junit.jupiter.api.Test; + +public class RangerUtilsTest { + + @Test + void toUserInfoIncludesUserDefinedPrincipalProperties() { + PolarisPrincipal principal = + PolarisPrincipal.of( + "alice", + Map.of( + PolarisPrincipal.PRINCIPAL_ENTITY_ATTRIBUTE_KEY, + new PrincipalEntity.Builder() + .setName("alice") + .setProperties(Map.of("department", "finance")) + .setClientId("client-id-123") + .build()), + Set.of("admin")); + + RangerUserInfo userInfo = RangerUtils.toUserInfo(principal); + + assertThat(userInfo.getName()).isEqualTo("alice"); + assertThat(userInfo.getRoles()).containsExactly("admin"); + assertThat(userInfo.getAttributes()).containsEntry("department", "finance"); + assertThat(userInfo.getAttributes()) + .containsEntry(PolarisEntityConstants.getClientIdPropertyName(), "client-id-123"); + } + + @Test + void toUserInfoPrefersInternalPropertiesOnCollision() { + PolarisPrincipal principal = + PolarisPrincipal.of( + "alice", + Map.of( + PolarisPrincipal.PRINCIPAL_ENTITY_ATTRIBUTE_KEY, + new PrincipalEntity.Builder() + .setName("alice") + .setProperties( + Map.of(PolarisEntityConstants.getClientIdPropertyName(), "user-value")) + .setClientId("internal-value") + .build()), + Set.of("admin")); + + RangerUserInfo userInfo = RangerUtils.toUserInfo(principal); + + assertThat(userInfo.getAttributes()) + .containsEntry(PolarisEntityConstants.getClientIdPropertyName(), "internal-value"); + } +} 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 4d8a9d58bd..49484e4522 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 @@ -438,11 +438,37 @@ void testInputIdentityAttributesPassedThrough() { assertThat(result.getAttribute("custom-key", String.class)).hasValue("custom-value"); } + @Test + void testPrincipalEntityAttributeContainsUserProperties() { + // Given: a principal with user-defined properties + PrincipalEntity entity = + createPrincipal("principal-with-properties", Map.of("department", "finance")); + PolarisCredential credentials = + PolarisCredential.of( + null, entity.getName(), Set.of(DefaultAuthenticator.PRINCIPAL_ROLE_ALL)); + + // When: authenticating the principal + PolarisPrincipal result = authenticator.authenticate(identityFor(credentials)); + + // Then: the principal entity attribute must include the user-defined properties + PrincipalEntity entityAttr = + result + .getAttribute(PolarisPrincipal.PRINCIPAL_ENTITY_ATTRIBUTE_KEY, PrincipalEntity.class) + .orElseThrow(); + assertThat(entityAttr.getPropertiesAsMap()).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(