From 463aab7e1136e08d84472e0dd95dfa396e38d1ab Mon Sep 17 00:00:00 2001 From: Alexandre Dutra Date: Thu, 14 Aug 2025 18:52:27 +0200 Subject: [PATCH] Remove numeric identifier from PolarisPrincipal This change removes the requirement for Polaris principals to have a numeric identifier, by removing the only sites where such an identifier was required: - In the `Resolver`. Instead, the `Resolver` now performs a lookup by principal name. - In `PolarisAdminService`. Instead, the code now compares the principal name against the entity name. Note: the lookup in the `Resolver` is still necessary, because the `Resolver` also needs to fetch the grant records. --- .../polaris/core/auth/PolarisPrincipal.java | 24 ++++----------- .../core/persistence/resolver/Resolver.java | 6 +--- .../service/admin/PolarisAdminService.java | 30 +++++++++++++++---- .../auth/PersistedPolarisPrincipal.java | 6 ---- .../service/admin/PolarisAuthzTestBase.java | 5 ++-- 5 files changed, 34 insertions(+), 37 deletions(-) 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 cfa5612a32..e3be050147 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 @@ -39,26 +39,19 @@ public interface PolarisPrincipal extends Principal { * @param roles the set of roles associated with the principal */ static PolarisPrincipal of(PrincipalEntity principalEntity, Set roles) { - return of( - principalEntity.getId(), - principalEntity.getName(), - principalEntity.getInternalPropertiesAsMap(), - roles); + return of(principalEntity.getName(), principalEntity.getInternalPropertiesAsMap(), roles); } /** * Creates a new instance of {@link PolarisPrincipal} with the specified ID, name, roles, and * properties. * - * @param id the unique identifier of the principal * @param name the name of the principal * @param properties additional properties associated with the principal * @param roles the set of roles associated with the principal */ - static PolarisPrincipal of( - long id, String name, Map properties, Set roles) { + static PolarisPrincipal of(String name, Map properties, Set roles) { return ImmutablePolarisPrincipal.builder() - .id(id) .name(name) .properties(properties) .roles(roles) @@ -66,16 +59,11 @@ static PolarisPrincipal of( } /** - * Returns the unique identifier of the principal. + * Returns the set of activated principal role names. * - *

This identifier is used to uniquely identify the principal within a Polaris realm. - */ - long getId(); - - /** - * Returns the set of activated principal role names. Activated role names are the roles that were - * explicitly requested by the client when authenticating, through JWT claims or other means. It - * may be a subset of the roles that the principal has in the system. + *

Activated role names are the roles that were explicitly requested by the client when + * authenticating, through JWT claims or other means. It may be a subset of the roles that the + * principal has in the system. */ Set getRoles(); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/Resolver.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/Resolver.java index df64d296a1..a4eaf3dfe6 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/Resolver.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/Resolver.java @@ -755,11 +755,7 @@ private ResolverStatus resolveCallerPrincipalAndPrincipalRoles( // resolve the principal, by name or id this.resolvedCallerPrincipal = - this.resolveById( - toValidate, - PolarisEntityType.PRINCIPAL, - PolarisEntityConstants.getNullId(), - polarisPrincipal.getId()); + this.resolveByName(toValidate, PolarisEntityType.PRINCIPAL, polarisPrincipal.getName()); // if the principal was not found, we can end right there if (this.resolvedCallerPrincipal == null diff --git a/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java b/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java index 682a17389e..090f7d3d20 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java @@ -251,11 +251,8 @@ private void authorizeBasicTopLevelEntityOperationOrThrow( PolarisResolvedPathWrapper topLevelEntityWrapper = resolutionManifest.getResolvedTopLevelEntity(topLevelEntityName, entityType); - // TODO: If we do add more "self" privilege operations for PRINCIPAL targets this should - // be extracted into an EnumSet and/or pushed down into PolarisAuthorizer. - if (topLevelEntityWrapper.getResolvedLeafEntity().getEntity().getId() - == polarisPrincipal.getId() - && (op.equals(PolarisAuthorizableOperation.ROTATE_CREDENTIALS))) { + PolarisEntity entity = topLevelEntityWrapper.getResolvedLeafEntity().getEntity(); + if (isSelfEntity(entity) && isSelfOperation(op)) { LOGGER .atDebug() .addKeyValue("principalName", topLevelEntityName) @@ -270,6 +267,29 @@ private void authorizeBasicTopLevelEntityOperationOrThrow( null /* secondary */); } + /** + * Returns true if the target entity is the same as the current authenticated {@link + * PolarisPrincipal}. + */ + private boolean isSelfEntity(PolarisEntity entity) { + // Entity name is unique for (realm_id, catalog_id, parent_id, type_code), + // which is reduced to (realm_id, type_code) for top-level entities; + // so there can be only one principal with a given name inside any realm. + return entity.getType() == PolarisEntityType.PRINCIPAL + && entity.getName().equals(polarisPrincipal.getName()); + } + + /** + * Returns true if the operation is a "self" operation, that is, an operation that is being + * performed by the principal on itself. + * + *

TODO: If we do add more "self" privilege operations for PRINCIPAL targets this should be + * extracted into an EnumSet and/or pushed down into PolarisAuthorizer. + */ + private static boolean isSelfOperation(PolarisAuthorizableOperation op) { + return op.equals(PolarisAuthorizableOperation.ROTATE_CREDENTIALS); + } + private void authorizeBasicCatalogRoleOperationOrThrow( PolarisAuthorizableOperation op, String catalogName, String catalogRoleName) { resolutionManifest = diff --git a/runtime/service/src/main/java/org/apache/polaris/service/auth/PersistedPolarisPrincipal.java b/runtime/service/src/main/java/org/apache/polaris/service/auth/PersistedPolarisPrincipal.java index 5597366645..c9b66c03c4 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/auth/PersistedPolarisPrincipal.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/auth/PersistedPolarisPrincipal.java @@ -52,12 +52,6 @@ public String getName() { return getEntity().getName(); } - @Value.Derived - @Override - public long getId() { - return getEntity().getId(); - } - @Value.Lazy @Override public Map getProperties() { diff --git a/runtime/service/src/test/java/org/apache/polaris/service/admin/PolarisAuthzTestBase.java b/runtime/service/src/test/java/org/apache/polaris/service/admin/PolarisAuthzTestBase.java index fee88c25a5..1140e3bf66 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/admin/PolarisAuthzTestBase.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/admin/PolarisAuthzTestBase.java @@ -399,9 +399,8 @@ protected static void assertSuccess(BaseResult result) { protected @Nonnull Set loadPrincipalRolesNames(PolarisPrincipal p) { PolarisBaseEntity principal = metaStoreManager - .loadEntity( - callContext.getPolarisCallContext(), 0L, p.getId(), PolarisEntityType.PRINCIPAL) - .getEntity(); + .findPrincipalByName(callContext.getPolarisCallContext(), p.getName()) + .orElseThrow(); return metaStoreManager .loadGrantsToGrantee(callContext.getPolarisCallContext(), principal) .getGrantRecords()