Skip to content

Commit 463aab7

Browse files
committed
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.
1 parent 4885d77 commit 463aab7

File tree

5 files changed

+34
-37
lines changed

5 files changed

+34
-37
lines changed

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

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -39,43 +39,31 @@ public interface PolarisPrincipal extends Principal {
3939
* @param roles the set of roles associated with the principal
4040
*/
4141
static PolarisPrincipal of(PrincipalEntity principalEntity, Set<String> roles) {
42-
return of(
43-
principalEntity.getId(),
44-
principalEntity.getName(),
45-
principalEntity.getInternalPropertiesAsMap(),
46-
roles);
42+
return of(principalEntity.getName(), principalEntity.getInternalPropertiesAsMap(), roles);
4743
}
4844

4945
/**
5046
* Creates a new instance of {@link PolarisPrincipal} with the specified ID, name, roles, and
5147
* properties.
5248
*
53-
* @param id the unique identifier of the principal
5449
* @param name the name of the principal
5550
* @param properties additional properties associated with the principal
5651
* @param roles the set of roles associated with the principal
5752
*/
58-
static PolarisPrincipal of(
59-
long id, String name, Map<String, String> properties, Set<String> roles) {
53+
static PolarisPrincipal of(String name, Map<String, String> properties, Set<String> roles) {
6054
return ImmutablePolarisPrincipal.builder()
61-
.id(id)
6255
.name(name)
6356
.properties(properties)
6457
.roles(roles)
6558
.build();
6659
}
6760

6861
/**
69-
* Returns the unique identifier of the principal.
62+
* Returns the set of activated principal role names.
7063
*
71-
* <p>This identifier is used to uniquely identify the principal within a Polaris realm.
72-
*/
73-
long getId();
74-
75-
/**
76-
* Returns the set of activated principal role names. Activated role names are the roles that were
77-
* explicitly requested by the client when authenticating, through JWT claims or other means. It
78-
* may be a subset of the roles that the principal has in the system.
64+
* <p>Activated role names are the roles that were explicitly requested by the client when
65+
* authenticating, through JWT claims or other means. It may be a subset of the roles that the
66+
* principal has in the system.
7967
*/
8068
Set<String> getRoles();
8169

polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/Resolver.java

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -755,11 +755,7 @@ private ResolverStatus resolveCallerPrincipalAndPrincipalRoles(
755755

756756
// resolve the principal, by name or id
757757
this.resolvedCallerPrincipal =
758-
this.resolveById(
759-
toValidate,
760-
PolarisEntityType.PRINCIPAL,
761-
PolarisEntityConstants.getNullId(),
762-
polarisPrincipal.getId());
758+
this.resolveByName(toValidate, PolarisEntityType.PRINCIPAL, polarisPrincipal.getName());
763759

764760
// if the principal was not found, we can end right there
765761
if (this.resolvedCallerPrincipal == null

runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -251,11 +251,8 @@ private void authorizeBasicTopLevelEntityOperationOrThrow(
251251
PolarisResolvedPathWrapper topLevelEntityWrapper =
252252
resolutionManifest.getResolvedTopLevelEntity(topLevelEntityName, entityType);
253253

254-
// TODO: If we do add more "self" privilege operations for PRINCIPAL targets this should
255-
// be extracted into an EnumSet and/or pushed down into PolarisAuthorizer.
256-
if (topLevelEntityWrapper.getResolvedLeafEntity().getEntity().getId()
257-
== polarisPrincipal.getId()
258-
&& (op.equals(PolarisAuthorizableOperation.ROTATE_CREDENTIALS))) {
254+
PolarisEntity entity = topLevelEntityWrapper.getResolvedLeafEntity().getEntity();
255+
if (isSelfEntity(entity) && isSelfOperation(op)) {
259256
LOGGER
260257
.atDebug()
261258
.addKeyValue("principalName", topLevelEntityName)
@@ -270,6 +267,29 @@ private void authorizeBasicTopLevelEntityOperationOrThrow(
270267
null /* secondary */);
271268
}
272269

270+
/**
271+
* Returns true if the target entity is the same as the current authenticated {@link
272+
* PolarisPrincipal}.
273+
*/
274+
private boolean isSelfEntity(PolarisEntity entity) {
275+
// Entity name is unique for (realm_id, catalog_id, parent_id, type_code),
276+
// which is reduced to (realm_id, type_code) for top-level entities;
277+
// so there can be only one principal with a given name inside any realm.
278+
return entity.getType() == PolarisEntityType.PRINCIPAL
279+
&& entity.getName().equals(polarisPrincipal.getName());
280+
}
281+
282+
/**
283+
* Returns true if the operation is a "self" operation, that is, an operation that is being
284+
* performed by the principal on itself.
285+
*
286+
* <p>TODO: If we do add more "self" privilege operations for PRINCIPAL targets this should be
287+
* extracted into an EnumSet and/or pushed down into PolarisAuthorizer.
288+
*/
289+
private static boolean isSelfOperation(PolarisAuthorizableOperation op) {
290+
return op.equals(PolarisAuthorizableOperation.ROTATE_CREDENTIALS);
291+
}
292+
273293
private void authorizeBasicCatalogRoleOperationOrThrow(
274294
PolarisAuthorizableOperation op, String catalogName, String catalogRoleName) {
275295
resolutionManifest =

runtime/service/src/main/java/org/apache/polaris/service/auth/PersistedPolarisPrincipal.java

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,6 @@ public String getName() {
5252
return getEntity().getName();
5353
}
5454

55-
@Value.Derived
56-
@Override
57-
public long getId() {
58-
return getEntity().getId();
59-
}
60-
6155
@Value.Lazy
6256
@Override
6357
public Map<String, String> getProperties() {

runtime/service/src/test/java/org/apache/polaris/service/admin/PolarisAuthzTestBase.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -399,9 +399,8 @@ protected static void assertSuccess(BaseResult result) {
399399
protected @Nonnull Set<String> loadPrincipalRolesNames(PolarisPrincipal p) {
400400
PolarisBaseEntity principal =
401401
metaStoreManager
402-
.loadEntity(
403-
callContext.getPolarisCallContext(), 0L, p.getId(), PolarisEntityType.PRINCIPAL)
404-
.getEntity();
402+
.findPrincipalByName(callContext.getPolarisCallContext(), p.getName())
403+
.orElseThrow();
405404
return metaStoreManager
406405
.loadGrantsToGrantee(callContext.getPolarisCallContext(), principal)
407406
.getGrantRecords()

0 commit comments

Comments
 (0)