This is an automated email from the ASF dual-hosted git repository.
emaynard pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/polaris.git
The following commit(s) were added to refs/heads/main by this push:
new dd223c1bd Make entity lookups by id honor the specified entity type
(#1401)
dd223c1bd is described below
commit dd223c1bd9a0ccfe2520d76e388f0e1940958737
Author: Alexandre Dutra <adu...@users.noreply.github.com>
AuthorDate: Wed Apr 23 19:25:55 2025 +0200
Make entity lookups by id honor the specified entity type (#1401)
* Make entity lookups by id honor the specified entity type
All implementations of
`TransactionalPersistence.lookupEntityInCurrentTxn()` are currently ignoring
the `typeCode` parameter completely and could potentially return an entity of
the wrong type.
This can become very concerning during authentication, since a principal
lookup could return some entity that is not a principal, and that would be
considered a successful authentication.
* review
---
.../PolarisEclipseLinkMetaStoreSessionImpl.java | 6 +-
.../impl/eclipselink/PolarisEclipseLinkStore.java | 12 ++--
.../TreeMapTransactionalPersistenceImpl.java | 7 +-
.../BasePolarisMetaStoreManagerTest.java | 6 ++
.../persistence/PolarisTestMetaStoreManager.java | 80 ++++++++++++++++++++++
5 files changed, 103 insertions(+), 8 deletions(-)
diff --git
a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java
b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java
index 14bb01d83..e19b0ef72 100644
---
a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java
+++
b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java
@@ -309,7 +309,8 @@ public class PolarisEclipseLinkMetaStoreSessionImpl extends
AbstractTransactiona
@Nonnull PolarisCallContext callCtx, @Nonnull PolarisEntityCore entity) {
// delete it
- this.store.deleteFromEntities(localSession.get(), entity.getCatalogId(),
entity.getId());
+ this.store.deleteFromEntities(
+ localSession.get(), entity.getCatalogId(), entity.getId(),
entity.getTypeCode());
}
/** {@inheritDoc} */
@@ -360,7 +361,8 @@ public class PolarisEclipseLinkMetaStoreSessionImpl extends
AbstractTransactiona
@Override
public @Nullable PolarisBaseEntity lookupEntityInCurrentTxn(
@Nonnull PolarisCallContext callCtx, long catalogId, long entityId, int
typeCode) {
- return ModelEntity.toEntity(this.store.lookupEntity(localSession.get(),
catalogId, entityId));
+ return ModelEntity.toEntity(
+ this.store.lookupEntity(localSession.get(), catalogId, entityId,
typeCode));
}
@Override
diff --git
a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java
b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java
index fc889656b..9ebb4e816 100644
---
a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java
+++
b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkStore.java
@@ -84,7 +84,8 @@ public class PolarisEclipseLinkStore {
diagnosticServices.check(session != null, "session_is_null");
checkInitialized();
- ModelEntity model = lookupEntity(session, entity.getCatalogId(),
entity.getId());
+ ModelEntity model =
+ lookupEntity(session, entity.getCatalogId(), entity.getId(),
entity.getTypeCode());
if (model != null) {
// Update if the same entity already exists
model.update(entity);
@@ -129,11 +130,11 @@ public class PolarisEclipseLinkStore {
session.persist(ModelGrantRecord.fromGrantRecord(grantRec));
}
- void deleteFromEntities(EntityManager session, long catalogId, long
entityId) {
+ void deleteFromEntities(EntityManager session, long catalogId, long
entityId, int typeCode) {
diagnosticServices.check(session != null, "session_is_null");
checkInitialized();
- ModelEntity model = lookupEntity(session, catalogId, entityId);
+ ModelEntity model = lookupEntity(session, catalogId, entityId, typeCode);
diagnosticServices.check(model != null, "entity_not_found");
session.remove(model);
@@ -203,14 +204,15 @@ public class PolarisEclipseLinkStore {
LOGGER.debug("All entities deleted.");
}
- ModelEntity lookupEntity(EntityManager session, long catalogId, long
entityId) {
+ ModelEntity lookupEntity(EntityManager session, long catalogId, long
entityId, long typeCode) {
diagnosticServices.check(session != null, "session_is_null");
checkInitialized();
return session
.createQuery(
- "SELECT m from ModelEntity m where m.catalogId=:catalogId and
m.id=:id",
+ "SELECT m from ModelEntity m where m.catalogId=:catalogId and
m.id=:id and m.typeCode=:typeCode",
ModelEntity.class)
+ .setParameter("typeCode", typeCode)
.setParameter("catalogId", catalogId)
.setParameter("id", entityId)
.getResultStream()
diff --git
a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java
b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java
index 822045673..e9cbd53f3 100644
---
a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java
+++
b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java
@@ -217,7 +217,12 @@ public class TreeMapTransactionalPersistenceImpl extends
AbstractTransactionalPe
@Override
public @Nullable PolarisBaseEntity lookupEntityInCurrentTxn(
@Nonnull PolarisCallContext callCtx, long catalogId, long entityId, int
typeCode) {
- return
this.store.getSliceEntities().read(this.store.buildKeyComposite(catalogId,
entityId));
+ PolarisBaseEntity entity =
+
this.store.getSliceEntities().read(this.store.buildKeyComposite(catalogId,
entityId));
+ if (entity != null && entity.getTypeCode() != typeCode) {
+ return null;
+ }
+ return entity;
}
/** {@inheritDoc} */
diff --git
a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java
b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java
index b37a57464..d11bed82d 100644
---
a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java
+++
b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java
@@ -273,6 +273,12 @@ public abstract class BasePolarisMetaStoreManagerTest {
polarisTestMetaStoreManager.testRename();
}
+ /** test entity lookup */
+ @Test
+ protected void testLookup() {
+ polarisTestMetaStoreManager.testLookup();
+ }
+
/** Test the set of functions for the entity cache */
@Test
protected void testEntityCache() {
diff --git
a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java
b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java
index 81b95ab39..b401d4efb 100644
---
a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java
+++
b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java
@@ -192,6 +192,24 @@ public class PolarisTestMetaStoreManager {
return entity;
}
+ /**
+ * Validate that the specified identity identified by the pair catalogId,
entityId does not exist.
+ *
+ * @param catalogId catalog id of that entity
+ * @param entityId the entity id
+ * @param expectedType its expected type
+ */
+ private void ensureNotExistsById(long catalogId, long entityId,
PolarisEntityType expectedType) {
+
+ PolarisBaseEntity entity =
+ polarisMetaStoreManager
+ .loadEntity(this.polarisCallContext, catalogId, entityId,
expectedType)
+ .getEntity();
+
+ // assert entity was not found
+ Assertions.assertThat(entity).isNull();
+ }
+
/**
* Check if the specified grant record exists
*
@@ -2564,6 +2582,68 @@ public class PolarisTestMetaStoreManager {
this.renameEntity(List.of(catalog, N1, N1_N2), N1_N2_T1, List.of(catalog,
N5), "T7");
}
+ /** Play with looking up entities */
+ public void testLookup() {
+ // load all principals
+ List<EntityNameLookupRecord> principals =
+ polarisMetaStoreManager
+ .listEntities(
+ this.polarisCallContext,
+ null,
+ PolarisEntityType.PRINCIPAL,
+ PolarisEntitySubType.NULL_SUBTYPE)
+ .getEntities();
+
+ // ensure not null, one element only
+ Assertions.assertThat(principals).isNotNull().hasSize(1);
+
+ // get catalog list information
+ EntityNameLookupRecord principalListInfo = principals.get(0);
+
+ PolarisBaseEntity principal =
+ this.ensureExistsById(
+ null,
+ principalListInfo.getId(),
+ true,
+ PolarisEntityConstants.getRootPrincipalName(),
+ PolarisEntityType.PRINCIPAL,
+ PolarisEntitySubType.NULL_SUBTYPE);
+
+ this.ensureNotExistsById(
+ PolarisEntityConstants.getNullId(), principal.getId(),
PolarisEntityType.PRINCIPAL_ROLE);
+ this.ensureNotExistsById(
+ PolarisEntityConstants.getNullId(), principal.getId(),
PolarisEntityType.CATALOG);
+ this.ensureNotExistsById(
+ PolarisEntityConstants.getNullId(), principal.getId(),
PolarisEntityType.CATALOG_ROLE);
+
+ // create new catalog
+ PolarisBaseEntity catalog =
+ new PolarisBaseEntity(
+ PolarisEntityConstants.getNullId(),
+
polarisMetaStoreManager.generateNewEntityId(this.polarisCallContext).getId(),
+ PolarisEntityType.CATALOG,
+ PolarisEntitySubType.NULL_SUBTYPE,
+ PolarisEntityConstants.getRootEntityId(),
+ "test");
+ CreateCatalogResult catalogCreated =
+ polarisMetaStoreManager.createCatalog(this.polarisCallContext,
catalog, List.of());
+ Assertions.assertThat(catalogCreated).isNotNull();
+ catalog = catalogCreated.getCatalog();
+
+ // now create all objects
+ PolarisBaseEntity N1 = this.createEntity(List.of(catalog),
PolarisEntityType.NAMESPACE, "N1");
+ PolarisBaseEntity N1_N2 =
+ this.createEntity(List.of(catalog, N1), PolarisEntityType.NAMESPACE,
"N2");
+ PolarisBaseEntity T1 =
+ this.createEntity(
+ List.of(catalog, N1, N1_N2),
+ PolarisEntityType.TABLE_LIKE,
+ PolarisEntitySubType.ICEBERG_TABLE,
+ "T1");
+
+ this.ensureNotExistsById(catalog.getId(), T1.getId(),
PolarisEntityType.NAMESPACE);
+ }
+
/** Test the set of functions for the entity cache */
public void testEntityCache() {
// create test catalog
