BUG: Fix filtering of unreadable referenced entities in queries

src/main/java/org/caosdb/server/database/backend/transaction/RetrieveFullEntityTransaction.java

@@ -27,7 +27,6 @@ package org.caosdb.server.database.backend.transaction;
 import java.util.LinkedList;
 import java.util.List;
 import org.apache.shiro.subject.Subject;
-import org.caosdb.server.accessControl.Principal;
 import org.caosdb.server.database.BackendTransaction;
 import org.caosdb.server.database.exceptions.EntityDoesNotExistException;
 import org.caosdb.server.datatype.CollectionValue;
@@ -40,7 +39,6 @@ import org.caosdb.server.entity.RetrieveEntity;
 import org.caosdb.server.entity.Role;
 import org.caosdb.server.entity.container.Container;
 import org.caosdb.server.entity.wrapper.Property;
-import org.caosdb.server.permissions.EntityACL;
 import org.caosdb.server.permissions.EntityPermission;
 import org.caosdb.server.query.Query;
 import org.caosdb.server.query.Query.Selection;
@@ -63,11 +61,10 @@ public class RetrieveFullEntityTransaction extends BackendTransaction {
   private final Container<? extends EntityInterface> container;
   private final Subject subject;
 
-  public RetrieveFullEntityTransaction(final EntityInterface entity) {
-    final Container<EntityInterface> c = new Container<>();
-    c.add(entity);
-    this.container = c;
-    this.subject = null;
+  @SuppressWarnings("unchecked")
+  public RetrieveFullEntityTransaction(final EntityInterface entity, Subject subject) {
+    this(new Container<>(), subject);
+    ((Container<EntityInterface>) this.container).add(entity);
   }
 
   public RetrieveFullEntityTransaction(
@@ -76,13 +73,8 @@ public class RetrieveFullEntityTransaction extends BackendTransaction {
     this.subject = subject;
   }
 
-  public RetrieveFullEntityTransaction(final Container<? extends EntityInterface> container) {
-    this.container = container;
-    this.subject = null;
-  }
-
-  public RetrieveFullEntityTransaction(final EntityID id) {
-    this(new RetrieveEntity(id));
+  public RetrieveFullEntityTransaction(final EntityID id, Subject subject) {
+    this(new RetrieveEntity(id), subject);
   }
 
   @Override
@@ -235,15 +227,11 @@ public class RetrieveFullEntityTransaction extends BackendTransaction {
     final RetrieveEntity ref = new RetrieveEntity(value.getId());
 
     if (this.subject != null) {
-      System.out.println(((Principal) this.subject.getPrincipal()).getUsername());
-
-      // final EntityInterface e =
-      // execute(new RetrieveSparseEntity(q.getKey(), null), query.getAccess()).getEntity();
-      // check whether the referenced entity may be retrieved
       // recursion! (Only for the matching selections)
       retrieveFullEntity(ref, getSubSelects(selections, propertyName));
-      final EntityACL entityACL = ref.getEntityACL();
-      if (!entityACL.isPermitted(this.subject, EntityPermission.RETRIEVE_ENTITY)) {
+
+      // check whether the referenced entity is readable
+      if (!ref.getEntityACL().isPermitted(this.subject, EntityPermission.RETRIEVE_ENTITY)) {
         return;
       }
       value.setEntity(ref, true);

src/main/java/org/caosdb/server/entity/xml/EntityToElementStrategy.java

@@ -169,14 +169,16 @@ public class EntityToElementStrategy implements ToElementStrategy {
         // processing SELECT Queries.
         final EntityInterface ref = ((ReferenceValue) entity.getValue()).getEntity();
         if (ref != null) {
-          if (entity.hasDatatype()) {
-            setDatatype(entity, element);
-          }
           ref.addToElement(element, serializeFieldStrategy);
-          // the referenced entity has been appended. Return here to suppress
-          // adding the reference id as well.
-          return;
+        } else {
+          entity.getValue().addToElement(element);
+        }
+        if (entity.hasDatatype()) {
+          setDatatype(entity, element);
         }
+        // the referenced entity has been appended. Return here to suppress
+        // adding the reference id as well.
+        return;
       } else if (entity.isReferenceList() && serializeFieldStrategy.isToBeSet("_referenced")) {
         // Append the all referenced entities. This needs to be done when we are
         // processing SELECT Queries.

src/main/java/org/caosdb/server/query/POV.java

@@ -405,6 +405,12 @@ public class POV implements EntityFilterInterface {
     }
     final long t2 = System.currentTimeMillis();
     query.addBenchmark(measurement(".initPOVRefidsTable()"), t2 - t1);
+
+    if (this.refIdsTable != null) {
+      query.getQuery().filterIntermediateResult(this.refIdsTable);
+    }
+    final long t3 = System.currentTimeMillis();
+    query.addBenchmark(measurement(".filterRefidsWithoutRetrievePermission"), t3 - t2);
     try (PreparedStatement stmt =
         query.getConnection().prepareCall("call initPOVPropertiesTable(?,?,?)")) {
       // initPOVPropertiesTable(in pid INT UNSIGNED, in pname
@@ -455,12 +461,12 @@ public class POV implements EntityFilterInterface {
         }
       }
     }
-    final long t3 = System.currentTimeMillis();
-    query.addBenchmark(measurement(""), t3 - t2);
+    final long t4 = System.currentTimeMillis();
+    query.addBenchmark(measurement(""), t4 - t3);
 
     if (this.refIdsTable != null) {
       query.getQuery().applyQueryTemplates(query, this.refIdsTable);
-      query.addBenchmark(measurement(".applyQueryTemplates()"), System.currentTimeMillis() - t3);
+      query.addBenchmark(measurement(".applyQueryTemplates()"), System.currentTimeMillis() - t4);
     }
 
     if (hasSubProperty() && this.targetSet != null) {

src/main/java/org/caosdb/server/transaction/UpdateACL.java

@@ -67,7 +67,8 @@ public class UpdateACL extends Transaction<TransactionContainer>
       oldContainer.add(new UpdateEntity(e.getId(), null));
     }
 
-    RetrieveFullEntityTransaction t = new RetrieveFullEntityTransaction(oldContainer);
+    RetrieveFullEntityTransaction t =
+        new RetrieveFullEntityTransaction(oldContainer, getTransactor());
     execute(t, getAccess());
 
     // the entities in this container only have an id and an ACL. -> Replace

src/main/java/org/caosdb/server/transaction/WriteTransaction.java

@@ -165,10 +165,10 @@ public class WriteTransaction extends Transaction<WritableContainer>
 
     // Retrieve a container which contains all IDs of those entities
     // which are to be updated.
-    execute(new RetrieveFullEntityTransaction(oldContainer), getAccess());
+    execute(new RetrieveFullEntityTransaction(oldContainer, getTransactor()), getAccess());
 
     // Retrieve all entities which are to be deleted.
-    execute(new RetrieveFullEntityTransaction(deleteContainer), getAccess());
+    execute(new RetrieveFullEntityTransaction(deleteContainer, getTransactor()), getAccess());
 
     // Check if any updates are to be processed.
     for (final EntityInterface entity : getContainer()) {

src/test/java/org/caosdb/server/database/backend/transaction/RetrieveFullEntityTest.java

@@ -23,8 +23,21 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 
+import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
+import java.util.concurrent.Callable;
+import org.apache.shiro.authc.AuthenticationException;
+import org.apache.shiro.authc.AuthenticationToken;
+import org.apache.shiro.authz.AuthorizationException;
+import org.apache.shiro.authz.Permission;
+import org.apache.shiro.session.Session;
+import org.apache.shiro.subject.ExecutionException;
+import org.apache.shiro.subject.PrincipalCollection;
+import org.apache.shiro.subject.Subject;
+import org.caosdb.server.CaosDBServer;
+import org.caosdb.server.accessControl.Principal;
 import org.caosdb.server.datatype.ReferenceValue;
 import org.caosdb.server.entity.Entity;
 import org.caosdb.server.entity.EntityID;
@@ -32,15 +45,168 @@ import org.caosdb.server.entity.EntityInterface;
 import org.caosdb.server.entity.RetrieveEntity;
 import org.caosdb.server.entity.wrapper.Property;
 import org.caosdb.server.entity.xml.PropertyToElementStrategyTest;
+import org.caosdb.server.permissions.EntityACL;
 import org.caosdb.server.query.Query.Selection;
+import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 
 public class RetrieveFullEntityTest {
 
+  @BeforeAll
+  public static void setup() throws IOException {
+    CaosDBServer.initServerProperties();
+  }
+
   @Test
   public void testRetrieveSubEntities() {
     final RetrieveFullEntityTransaction r =
-        new RetrieveFullEntityTransaction(new EntityID("0")) {
+        new RetrieveFullEntityTransaction(
+            new EntityID("0"),
+            new Subject() {
+
+              @Override
+              public Object getPrincipal() {
+                return new Principal("Bla", "Blub");
+              }
+
+              @Override
+              public PrincipalCollection getPrincipals() {
+                return null;
+              }
+
+              @Override
+              public boolean isPermitted(String permission) {
+                assertEquals(
+                    permission, org.caosdb.server.permissions.EntityPermission.RETRIEVE_ENTITY);
+                return true;
+              }
+
+              @Override
+              public boolean isPermitted(Permission permission) {
+                return false;
+              }
+
+              @Override
+              public boolean[] isPermitted(String... permissions) {
+                return null;
+              }
+
+              @Override
+              public boolean[] isPermitted(List<Permission> permissions) {
+                return null;
+              }
+
+              @Override
+              public boolean isPermittedAll(String... permissions) {
+                return false;
+              }
+
+              @Override
+              public boolean isPermittedAll(Collection<Permission> permissions) {
+                return false;
+              }
+
+              @Override
+              public void checkPermission(String permission) throws AuthorizationException {}
+
+              @Override
+              public void checkPermission(Permission permission) throws AuthorizationException {}
+
+              @Override
+              public void checkPermissions(String... permissions) throws AuthorizationException {}
+
+              @Override
+              public void checkPermissions(Collection<Permission> permissions)
+                  throws AuthorizationException {}
+
+              @Override
+              public boolean hasRole(String roleIdentifier) {
+                return false;
+              }
+
+              @Override
+              public boolean[] hasRoles(List<String> roleIdentifiers) {
+                return null;
+              }
+
+              @Override
+              public boolean hasAllRoles(Collection<String> roleIdentifiers) {
+                return false;
+              }
+
+              @Override
+              public void checkRole(String roleIdentifier) throws AuthorizationException {}
+
+              @Override
+              public void checkRoles(Collection<String> roleIdentifiers)
+                  throws AuthorizationException {}
+
+              @Override
+              public void checkRoles(String... roleIdentifiers) throws AuthorizationException {}
+
+              @Override
+              public void login(AuthenticationToken token) throws AuthenticationException {}
+
+              @Override
+              public boolean isAuthenticated() {
+                return false;
+              }
+
+              @Override
+              public boolean isRemembered() {
+                return false;
+              }
+
+              @Override
+              public Session getSession() {
+                return null;
+              }
+
+              @Override
+              public Session getSession(boolean create) {
+                return null;
+              }
+
+              @Override
+              public void logout() {}
+
+              @Override
+              public <V> V execute(Callable<V> callable) throws ExecutionException {
+                return null;
+              }
+
+              @Override
+              public void execute(Runnable runnable) {}
+
+              @Override
+              public <V> Callable<V> associateWith(Callable<V> callable) {
+                return null;
+              }
+
+              @Override
+              public Runnable associateWith(Runnable runnable) {
+                return null;
+              }
+
+              @Override
+              public void runAs(PrincipalCollection principals)
+                  throws NullPointerException, IllegalStateException {}
+
+              @Override
+              public boolean isRunAs() {
+                return false;
+              }
+
+              @Override
+              public PrincipalCollection getPreviousPrincipals() {
+                return null;
+              }
+
+              @Override
+              public PrincipalCollection releaseRunAs() {
+                return null;
+              }
+            }) {
 
           /** Mock-up */
           @Override
@@ -53,6 +219,7 @@ public class RetrieveFullEntityTest {
             assertEquals("description", selections.get(0).getSelector());
 
             e.setDescription("A heart-shaped window.");
+            e.setEntityACL(new EntityACL());
           }
           ;
         };