Switch principal attributes to AttributeMap - #5140
Conversation
| * | ||
| * <p>Note: callers must never assume that this attribute is present. | ||
| */ | ||
| String PRINCIPAL_ENTITY_ATTRIBUTE_KEY = "org.apache.polaris.core.auth.PRINCIPAL_ENTITY"; |
There was a problem hiding this comment.
Moved to a separate PolarisPrincipalAttributes class.
|
This PR is ready for review. |
There was a problem hiding this comment.
Pull request overview
Updates the PolarisPrincipal attribute model from a raw Map<String, Object> to a type-safe AttributeMap, introducing shared typed keys and propagating the change through core auth/persistence logic, runtime service authentication, and extension authorizer tests.
Changes:
- Switch
PolarisPrincipalto carry anAttributeMapand introducePolarisPrincipalAttributestyped keys for principal entity / role-all / JWT attributes. - Update runtime/service authentication and core resolver/auth preconditions to use typed attribute access.
- Migrate affected tests and extension integrations (OPA, Ranger, Kafka events) to the new attribute model and add coverage for defensive copying.
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| runtime/service/src/testFixtures/java/org/apache/polaris/service/TestServices.java | Update test fixture principal construction to use ImmutableAttributeMap. |
| runtime/service/src/test/java/org/apache/polaris/service/events/listeners/PolarisPersistenceEventListenerTest.java | Use AttributeMap.EMPTY when building principals in event listener tests. |
| runtime/service/src/test/java/org/apache/polaris/service/events/listeners/opentelemetry/OpenTelemetryEventListenerTest.java | Use AttributeMap.EMPTY for metadata user principals. |
| runtime/service/src/test/java/org/apache/polaris/service/events/jsonEventListener/aws/cloudwatch/AwsCloudWatchEventListenerTest.java | Replace empty Map principal attributes with AttributeMap.EMPTY. |
| runtime/service/src/test/java/org/apache/polaris/service/catalog/RootPrincipalAugmentor.java | Build root principal attributes using PolarisPrincipalAttributes + ImmutableAttributeMap. |
| runtime/service/src/test/java/org/apache/polaris/service/catalog/policy/PolicyCatalogHandlerAuthzTest.java | Migrate principal attributes to typed AttributeMap in policy authz tests. |
| runtime/service/src/test/java/org/apache/polaris/service/catalog/policy/AbstractPolicyCatalogTest.java | Migrate authenticated root principal attributes to ImmutableAttributeMap. |
| runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalogTest.java | Use AttributeMap.EMPTY for test principals. |
| runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerTest.java | Use AttributeMap.EMPTY for handler principal. |
| runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandlerFineGrainedDisabledTest.java | Use PolarisPrincipalAttributes keys in authz-disabled handler tests. |
| runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractLocalIcebergCatalogViewTest.java | Migrate authenticated root principal to typed attributes. |
| runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractIcebergCatalogHandlerAuthzTest.java | Update multiple handler principal constructions to typed attribute maps. |
| runtime/service/src/test/java/org/apache/polaris/service/catalog/generic/PolarisGenericTableCatalogHandlerAuthzTest.java | Update generic handler authz tests to typed attribute maps. |
| runtime/service/src/test/java/org/apache/polaris/service/catalog/generic/AbstractPolarisGenericTableCatalogTest.java | Update generic catalog test base to typed principal attributes. |
| runtime/service/src/test/java/org/apache/polaris/service/auth/DefaultAuthenticatorTest.java | Update assertions and principal creation for AttributeMap + typed keys. |
| runtime/service/src/test/java/org/apache/polaris/service/auth/AuthenticatingAugmentorTest.java | Adjust augmentor test to reflect principal attributes being an AttributeMap. |
| runtime/service/src/test/java/org/apache/polaris/service/admin/PolarisAuthzTestBase.java | Update authenticated root principal to typed attributes. |
| runtime/service/src/test/java/org/apache/polaris/service/admin/PolarisAdminServiceAuthzTest.java | Update admin service authz tests to typed principal attributes. |
| runtime/service/src/test/java/org/apache/polaris/service/admin/ManagementServiceTest.java | Update management service setup principal to typed attribute map. |
| runtime/service/src/main/java/org/apache/polaris/service/auth/DefaultAuthenticator.java | Switch resolved principal attributes from Map to AttributeMap and use typed keys. |
| runtime/service/src/main/java/org/apache/polaris/service/auth/AuthenticatingAugmentor.java | Convert principal AttributeMap back into Quarkus SecurityIdentity attributes. |
| polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BaseResolverTest.java | Update resolver test fixture principal creation to typed attributes. |
| polaris-core/src/test/java/org/apache/polaris/core/storage/aws/AwsCredentialsStorageIntegrationTest.java | Use AttributeMap.EMPTY for test principal. |
| polaris-core/src/test/java/org/apache/polaris/core/persistence/ResolverTest.java | Replace empty Map principal attributes with AttributeMap.EMPTY. |
| polaris-core/src/test/java/org/apache/polaris/core/persistence/PolarisResolutionManifestLookupKeyTest.java | Replace empty Map principal attributes with AttributeMap.EMPTY. |
| polaris-core/src/test/java/org/apache/polaris/core/auth/PolarisPrincipalTest.java | Add test coverage for defensive copy/immutability of principal attributes. |
| polaris-core/src/test/java/org/apache/polaris/core/auth/PolarisAuthorizerImplTest.java | Replace empty Map principal attributes with AttributeMap.EMPTY. |
| polaris-core/src/test/java/org/apache/polaris/core/auth/AuthorizationRequestTest.java | Replace empty Map principal attributes with AttributeMap.EMPTY. |
| polaris-core/src/test/java/org/apache/polaris/core/auth/AuthorizationPreConditionsTest.java | Update principal construction and empty-attrs usage for new attribute model. |
| polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/Resolver.java | Update role-all lookup to use typed AttributeMap accessors. |
| polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisPrincipalAttributes.java | Introduce typed attribute keys for principal-related attributes. |
| polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisPrincipal.java | Change principal attributes to AttributeMap, remove string-key constants and typed getAttribute helper. |
| polaris-core/src/main/java/org/apache/polaris/core/auth/AuthorizationPreConditions.java | Update credential-rotation precondition checks to use typed principal attributes. |
| extensions/events/kafka/src/test/java/org/apache/polaris/extensions/events/kafka/KafkaEventListenerTest.java | Replace empty Map principal attributes with AttributeMap.EMPTY. |
| extensions/auth/ranger/src/test/java/org/apache/polaris/extension/auth/ranger/RangerPolarisAuthorizerTest.java | Update Ranger principal deserialization test to use AttributeMap.EMPTY. |
| extensions/auth/ranger/src/main/java/org/apache/polaris/extension/auth/ranger/utils/RangerUtils.java | Update principal-entity lookup to use typed attribute keys. |
| extensions/auth/opa/src/test/java/org/apache/polaris/extension/auth/opa/OpaPolarisAuthorizerTest.java | Migrate OPA tests to typed attribute maps (including custom user attributes). |
| extensions/auth/opa/src/test/java/org/apache/polaris/extension/auth/opa/OpaPolarisAuthorizerFactoryTest.java | Migrate OPA factory tests to AttributeMap-backed principals. |
flyingImer
left a comment
There was a problem hiding this comment.
I guess I'm a bit late to this topic. I wonder is the intent supporting ABAC authz? do we have any pointer to the design doc that can better navigate me throughout prs?
The intent is not to support ABAC, but just to introduce a type-safe mechanism to retrieve principal attributes. Principal attributes predate this PR. Granted, one could leverage principal attributes to implement some form of ABAC, but this PR does not make any visible step in that direction. |
Thanks Alex for the quick turnaround. In that case, could you please point me or share on why we need this type-safe mechanism change? I noticed #5139, but not quite caught up on why |
Simply to avoid unsafe type casts when retrieving attributes. |
Checklist
CHANGELOG.md(if needed)site/content/in-dev/unreleased(if needed)