Add getReadableIds and use in filterReadAccess #277

src/main/config/run.properties.example

@@ -45,6 +45,10 @@ log.list = SESSION WRITE READ INFO
 # Lucene
 lucene.url = https://localhost:8181
 lucene.populateBlockSize = 10000
+# Recommend setting lucene.searchBlockSize equal to maxIdsInQuery, so that all Lucene results can be authorised at once
+# If lucene.searchBlockSize > maxIdsInQuery, then multiple auth checks may be needed for a single search to Lucene
+# The optimal value depends on how likely a user's auth request fails: larger values are more efficient when rejection is more likely
+lucene.searchBlockSize = 1000
 lucene.directory = ${HOME}/data/icat/lucene
 lucene.backlogHandlerIntervalSeconds = 60
 lucene.enqueuedRequestIntervalSeconds = 5

src/main/java/org/icatproject/core/entity/EntityBaseBean.java

@@ -31,14 +31,15 @@
 import org.icatproject.core.manager.EntityInfoHandler;
 import org.icatproject.core.manager.EntityInfoHandler.Relationship;
 import org.icatproject.core.manager.GateKeeper;
+import org.icatproject.core.manager.HasEntityId;
 import org.icatproject.core.manager.LuceneManager;
 import org.icatproject.core.parser.IncludeClause.Step;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 @SuppressWarnings("serial")
 @MappedSuperclass
-public abstract class EntityBaseBean implements Serializable {
+public abstract class EntityBaseBean implements HasEntityId, Serializable {
 
  private static final EntityInfoHandler eiHandler = EntityInfoHandler.getInstance();
 

src/main/java/org/icatproject/core/manager/EntityBeanManager.java

@@ -148,6 +148,7 @@ public enum PersistMode {
  private boolean luceneActive;
 
  private int maxEntities;
+ private int luceneSearchBlockSize;
 
  private long exportCacheSize;
  private Set<String> rootUserNames;
@@ -781,27 +782,37 @@ private void exportTable(String beanName, Set<Long> ids, OutputStream output,
  }
  }
 
- private void filterReadAccess(List<ScoredEntityBaseBean> results, List<ScoredEntityBaseBean> allResults,
+ private void filterReadAccess(List<ScoredEntityBaseBean> acceptedResults, List<ScoredEntityBaseBean> newResults,
  int maxCount, String userId, EntityManager manager, Class<? extends EntityBaseBean> klass)
  throws IcatException {
 
- logger.debug("Got " + allResults.size() + " results from Lucene");
- for (ScoredEntityBaseBean sr : allResults) {
- long entityId = sr.getEntityBaseBeanId();
- EntityBaseBean beanManaged = manager.find(klass, entityId);
- if (beanManaged != null) {
- try {
- gateKeeper.performAuthorisation(userId, beanManaged, AccessType.READ, manager);
- results.add(new ScoredEntityBaseBean(entityId, sr.getScore()));
- if (results.size() > maxEntities) {
+ logger.debug("Got " + newResults.size() + " results from Lucene");
+ Set<Long> allowedIds = gateKeeper.getReadableIds(userId, newResults, klass.getSimpleName(), manager);
+ if (allowedIds == null) {
+ // A null result means there are no restrictions on the readable ids, so add as
+ // many newResults as we need to reach maxCount
+ int needed = maxCount - acceptedResults.size();
+ if (newResults.size() > needed) {
+ acceptedResults.addAll(newResults.subList(0, needed));
+ } else {
+ acceptedResults.addAll(newResults);
+ }
+ if (acceptedResults.size() > maxEntities) {
+ throw new IcatException(IcatExceptionType.VALIDATION,
+ "attempt to return more than " + maxEntities + " entities");
+ }
+ } else {
+ // Otherwise, add results in order until we reach maxCount
+ for (ScoredEntityBaseBean newResult : newResults) {
+ if (allowedIds.contains(newResult.getId())) {
+ acceptedResults.add(newResult);
+ if (acceptedResults.size() > maxEntities) {
  throw new IcatException(IcatExceptionType.VALIDATION,
  "attempt to return more than " + maxEntities + " entities");
  }
- if (results.size() == maxCount) {
+ if (acceptedResults.size() == maxCount) {
  break;
  }
- } catch (IcatException e) {
- // Nothing to do
  }
  }
  }
@@ -1154,6 +1165,7 @@ void init() {
  notificationRequests = propertyHandler.getNotificationRequests();
  luceneActive = lucene.isActive();
  maxEntities = propertyHandler.getMaxEntities();
+ luceneSearchBlockSize = propertyHandler.getLuceneSearchBlockSize();
  exportCacheSize = propertyHandler.getImportCacheSize();
  rootUserNames = propertyHandler.getRootUserNames();
  key = propertyHandler.getKey();
@@ -1398,11 +1410,7 @@ public List<ScoredEntityBaseBean> luceneDatafiles(String userName, String user,
  LuceneSearchResult last = null;
  Long uid = null;
  List<ScoredEntityBaseBean> allResults = Collections.emptyList();
- /*
- * As results may be rejected and maxCount may be 1 ensure that we
- * don't make a huge number of calls to Lucene
- */
- int blockSize = Math.max(1000, maxCount);
+ int blockSize = luceneSearchBlockSize;
 
  do {
  if (last == null) {
@@ -1423,7 +1431,7 @@ public List<ScoredEntityBaseBean> luceneDatafiles(String userName, String user,
  try (JsonGenerator gen = Json.createGenerator(baos).writeStartObject()) {
  gen.write("userName", userName);
  if (results.size() > 0) {
- gen.write("entityId", results.get(0).getEntityBaseBeanId());
+ gen.write("entityId", results.get(0).getId());
  }
  gen.writeEnd();
  }
@@ -1441,11 +1449,7 @@ public List<ScoredEntityBaseBean> luceneDatasets(String userName, String user, S
  LuceneSearchResult last = null;
  Long uid = null;
  List<ScoredEntityBaseBean> allResults = Collections.emptyList();
- /*
- * As results may be rejected and maxCount may be 1 ensure that we
- * don't make a huge number of calls to Lucene
- */
- int blockSize = Math.max(1000, maxCount);
+ int blockSize = luceneSearchBlockSize;
 
  do {
  if (last == null) {
@@ -1465,7 +1469,7 @@ public List<ScoredEntityBaseBean> luceneDatasets(String userName, String user, S
  try (JsonGenerator gen = Json.createGenerator(baos).writeStartObject()) {
  gen.write("userName", userName);
  if (results.size() > 0) {
- gen.write("entityId", results.get(0).getEntityBaseBeanId());
+ gen.write("entityId", results.get(0).getId());
  }
  gen.writeEnd();
  }
@@ -1492,11 +1496,7 @@ public List<ScoredEntityBaseBean> luceneInvestigations(String userName, String u
  LuceneSearchResult last = null;
  Long uid = null;
  List<ScoredEntityBaseBean> allResults = Collections.emptyList();
- /*
- * As results may be rejected and maxCount may be 1 ensure that we
- * don't make a huge number of calls to Lucene
- */
- int blockSize = Math.max(1000, maxCount);
+ int blockSize = luceneSearchBlockSize;
 
  do {
  if (last == null) {
@@ -1516,7 +1516,7 @@ public List<ScoredEntityBaseBean> luceneInvestigations(String userName, String u
  try (JsonGenerator gen = Json.createGenerator(baos).writeStartObject()) {
  gen.write("userName", userName);
  if (results.size() > 0) {
- gen.write("entityId", results.get(0).getEntityBaseBeanId());
+ gen.write("entityId", results.get(0).getId());
  }
  gen.writeEnd();
  }

src/main/java/org/icatproject/core/manager/GateKeeper.java

@@ -164,35 +164,102 @@ public Set<String> getPublicTables() {
  return publicTables;
  }
 
- public List<EntityBaseBean> getReadable(String userId, List<EntityBaseBean> beans, EntityManager manager) {
-
- if (beans.size() == 0) {
- return beans;
- }
-
- EntityBaseBean object = beans.get(0);
-
- Class<? extends EntityBaseBean> objectClass = object.getClass();
- String simpleName = objectClass.getSimpleName();
+ /**
+ * @param userId The user making the READ request
+ * @param simpleName The name of the requested entity type
+ * @param manager
+ * @return Returns a list of restrictions that apply to the requested entity
+ * type. If there are no restrictions, then returns null.
+ */
+ private List<String> getRestrictions(String userId, String simpleName, EntityManager manager) {
  if (rootUserNames.contains(userId)) {
  logger.info("\"Root\" user " + userId + " is allowed READ to " + simpleName);
- return beans;
+ return null;
  }
 
  TypedQuery<String> query = manager.createNamedQuery(Rule.INCLUDE_QUERY, String.class)
  .setParameter("member", userId).setParameter("bean", simpleName);
 
  List<String> restrictions = query.getResultList();
  logger.debug("Got " + restrictions.size() + " authz queries for READ by " + userId + " to a "
- + objectClass.getSimpleName());
+ + simpleName);
 
  for (String restriction : restrictions) {
  logger.debug("Query: " + restriction);
  if (restriction == null) {
  logger.info("Null restriction => READ permitted to " + simpleName);
- return beans;
+ return null;
+ }
+ }
+
+ return restrictions;
+ }
+
+ /**
+ * Returns a sub list of the passed entities that the user has READ access to.
+ * 
+ * @param userId The user making the READ request
+ * @param beans The entities the user wants to READ
+ * @param manager
+ * @return A list of entities the user has read access to
+ */
+ public List<EntityBaseBean> getReadable(String userId, List<EntityBaseBean> beans, EntityManager manager) {
+
+ if (beans.size() == 0) {
+ return beans;
+ }
+ EntityBaseBean object = beans.get(0);
+ Class<? extends EntityBaseBean> objectClass = object.getClass();
+ String simpleName = objectClass.getSimpleName();
+
+ List<String> restrictions = getRestrictions(userId, simpleName, manager);
+ if (restrictions == null) {
+ return beans;
+ }
+
+ Set<Long> readableIds = getReadableIds(userId, beans, restrictions, manager);
+
+ List<EntityBaseBean> results = new ArrayList<>();
+ for (EntityBaseBean bean : beans) {
+ if (readableIds.contains(bean.getId())) {
+ results.add(bean);
  }
  }
+ return results;
+ }
+
+ /**
+ * @param userId The user making the READ request
+ * @param entities The entities to check
+ * @param simpleName The name of the requested entity type
+ * @param manager
+ * @return Set of the ids that the user has read access to. If there are no
+ * restrictions, then returns null.
+ */
+ public Set<Long> getReadableIds(String userId, List<? extends HasEntityId> entities, String simpleName,
+ EntityManager manager) {
+
+ if (entities.size() == 0) {
+ return null;
+ }
+
+ List<String> restrictions = getRestrictions(userId, simpleName, manager);
+ if (restrictions == null) {
+ return null;
+ }
+
+ return getReadableIds(userId, entities, restrictions, manager);
+ }
+
+ /**
+ * @param userId The user making the READ request
+ * @param entities The entities to check
+ * @param restrictions The restrictions applying to the entities
+ * @param manager
+ * @return Set of the ids that the user has read access to
+ */
+ private Set<Long> getReadableIds(String userId, List<? extends HasEntityId> entities, List<String> restrictions,
+ EntityManager manager) {
 
  /*
  * IDs are processed in batches to avoid Oracle error: ORA-01795:
@@ -203,13 +270,13 @@ public List<EntityBaseBean> getReadable(String userId, List<EntityBaseBean> bean
  StringBuilder sb = null;
 
  int i = 0;
- for (EntityBaseBean bean : beans) {
+ for (HasEntityId entity : entities) {
  if (i == 0) {
  sb = new StringBuilder();
- sb.append(bean.getId());
+ sb.append(entity.getId());
  i = 1;
  } else {
- sb.append("," + bean.getId());
+ sb.append("," + entity.getId());
  i++;
  }
  if (i == maxIdsInQuery) {
@@ -222,27 +289,21 @@ public List<EntityBaseBean> getReadable(String userId, List<EntityBaseBean> bean
  idLists.add(sb.toString());
  }
 
- logger.debug("Check readability of " + beans.size() + " beans has been divided into " + idLists.size()
+ logger.debug("Check readability of " + entities.size() + " beans has been divided into " + idLists.size()
  + " queries.");
 
- Set<Long> ids = new HashSet<>();
+ Set<Long> readableIds = new HashSet<>();
  for (String idList : idLists) {
  for (String qString : restrictions) {
  TypedQuery<Long> q = manager.createQuery(qString.replace(":pkids", idList), Long.class);
  if (qString.contains(":user")) {
  q.setParameter("user", userId);
  }
- ids.addAll(q.getResultList());
+ readableIds.addAll(q.getResultList());
  }
  }
 
- List<EntityBaseBean> results = new ArrayList<>();
- for (EntityBaseBean bean : beans) {
- if (ids.contains(bean.getId())) {
- results.add(bean);
- }
- }
- return results;
+ return readableIds;
  }
 
  public Set<String> getRootUserNames() {

src/main/java/org/icatproject/core/manager/HasEntityId.java

@@ -0,0 +1,8 @@
+package org.icatproject.core.manager;
+
+/**
+ * Interface for objects representing entities that hold the entity id.
+ */
+public interface HasEntityId {
+ public Long getId();
+}

src/main/java/org/icatproject/core/manager/PropertyHandler.java

@@ -302,6 +302,7 @@ public int getLifetimeMinutes() {
  private String digestKey;
  private URL luceneUrl;
  private int lucenePopulateBlockSize;
+ private int luceneSearchBlockSize;
  private Path luceneDirectory;
  private long luceneBacklogHandlerIntervalMillis;
  private Map<String, String> cluster = new HashMap<>();
@@ -463,6 +464,9 @@ private void init() {
  lucenePopulateBlockSize = props.getPositiveInt("lucene.populateBlockSize");
  formattedProps.add("lucene.populateBlockSize" + " " + lucenePopulateBlockSize);
 
+ luceneSearchBlockSize = props.getPositiveInt("lucene.searchBlockSize");
+ formattedProps.add("lucene.searchBlockSize" + " " + luceneSearchBlockSize);
+
  luceneDirectory = props.getPath("lucene.directory");
  if (!luceneDirectory.toFile().isDirectory()) {
  String msg = luceneDirectory + " is not a directory";
@@ -612,6 +616,10 @@ public int getLucenePopulateBlockSize() {
  return lucenePopulateBlockSize;
  }
 
+ public int getLuceneSearchBlockSize() {
+ return luceneSearchBlockSize;
+ }
+
  public long getLuceneBacklogHandlerIntervalMillis() {
  return luceneBacklogHandlerIntervalMillis;
  }

src/main/java/org/icatproject/core/manager/ScoredEntityBaseBean.java

@@ -1,17 +1,17 @@
 package org.icatproject.core.manager;
 
-public class ScoredEntityBaseBean {
+public class ScoredEntityBaseBean implements HasEntityId {
 
- private long entityBaseBeanId;
+ private Long id;
  private float score;
 
- public ScoredEntityBaseBean(long id, float score) {
- this.entityBaseBeanId = id;
+ public ScoredEntityBaseBean(Long id, float score) {
+ this.id = id;
  this.score = score;
  }
 
- public long getEntityBaseBeanId() {
- return entityBaseBeanId;
+ public Long getId() {
+ return id;
  }
 
  public float getScore() {

src/main/java/org/icatproject/exposed/ICATRest.java

@@ -1108,7 +1108,7 @@ public String lucene(@Context HttpServletRequest request, @QueryParam("sessionId
  gen.writeStartArray();
  for (ScoredEntityBaseBean sb : objects) {
  gen.writeStartObject();
- gen.write("id", sb.getEntityBaseBeanId());
+ gen.write("id", sb.getId());
  gen.write("score", sb.getScore());
  gen.writeEnd();
  }