From 6c27f0d61105bef509a1751e752998f48b26d8ba Mon Sep 17 00:00:00 2001 From: Alan Kyffin Date: Tue, 3 Sep 2024 17:16:23 +0100 Subject: [PATCH] Read authentication rules in a separate transaction --- .../icatproject/core/manager/GateKeeper.java | 73 ++++++------------ .../core/manager/GateKeeperHelper.java | 74 +++++++++++++++++++ 2 files changed, 98 insertions(+), 49 deletions(-) create mode 100644 src/main/java/org/icatproject/core/manager/GateKeeperHelper.java diff --git a/src/main/java/org/icatproject/core/manager/GateKeeper.java b/src/main/java/org/icatproject/core/manager/GateKeeper.java index fe63d16d..1d0251d5 100644 --- a/src/main/java/org/icatproject/core/manager/GateKeeper.java +++ b/src/main/java/org/icatproject/core/manager/GateKeeper.java @@ -11,8 +11,6 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Set; -import java.util.concurrent.ConcurrentSkipListMap; -import java.util.concurrent.ConcurrentSkipListSet; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; @@ -40,7 +38,6 @@ import org.icatproject.core.IcatException; import org.icatproject.core.IcatException.IcatExceptionType; import org.icatproject.core.entity.EntityBaseBean; -import org.icatproject.core.entity.PublicStep; import org.icatproject.core.entity.Rule; import org.icatproject.core.manager.EntityInfoHandler.Relationship; import org.slf4j.Logger; @@ -73,17 +70,21 @@ public int compare(String o1, String o2) { @PersistenceContext(unitName = "icat") private EntityManager gateKeeperManager; + private final Logger logger = LoggerFactory.getLogger(GateKeeper.class); Marker fatal = MarkerFactory.getMarker("FATAL"); private int maxIdsInQuery; + @EJB + GateKeeperHelper gateKeeperHelper; + @EJB PropertyHandler propertyHandler; - private Map> publicSteps = new ConcurrentSkipListMap<>(); + private Map> publicSteps; - private Set publicTables = new ConcurrentSkipListSet<>(); + private Set publicTables; private Set rootUserNames; @@ -107,22 +108,16 @@ public int compare(String o1, String o2) { */ public boolean allowed(Relationship r) { String beanName = r.getDestinationBean().getSimpleName(); - // TODO by using the getter we can update the public tables if needed. - // Previous direct access meant we use out of date permissions, so why was there - // no update not here before? Needed for the publicSearchFields but if there's a - // reason for it to be publicTables and not getPublicTables can manually call - // update in SearchManager if (getPublicTables().contains(beanName)) { return true; } + String originBeanName = r.getOriginBean().getSimpleName(); - if (publicStepsStale) { - updatePublicSteps(); - } - Set fieldNames = publicSteps.get(originBeanName); + Set fieldNames = getPublicSteps().get(originBeanName); if (fieldNames != null && fieldNames.contains(r.getField().getName())) { return true; } + return false; } @@ -162,6 +157,13 @@ private void exit() { } } + private Map> getPublicSteps() { + if (publicStepsStale) { + updatePublicSteps(); + } + return publicSteps; + } + public Set getPublicTables() { if (publicTablesStale) { updatePublicTables(); @@ -191,10 +193,7 @@ private List getRestrictions(String userId, String simpleName, EntityMan return null; } - TypedQuery query = manager.createNamedQuery(Rule.INCLUDE_QUERY, String.class) - .setParameter("member", userId).setParameter("bean", simpleName); - - List restrictions = query.getResultList(); + List restrictions = gateKeeperHelper.getRules(Rule.INCLUDE_QUERY, userId, simpleName); logger.debug("Got " + restrictions.size() + " authz queries for READ by " + userId + " to a " + simpleName); @@ -381,9 +380,7 @@ public boolean isAccessAllowed(String user, EntityBaseBean object, AccessType ac } logger.debug("Checking " + qName + " " + user + " " + simpleName); - TypedQuery query = manager.createNamedQuery(qName, String.class).setParameter("member", user) - .setParameter("bean", simpleName); - List restrictions = query.getResultList(); + List restrictions = gateKeeperHelper.getRules(qName, user, simpleName); logger.debug( "Got " + restrictions.size() + " authz queries for " + access + " by " + user + " to a " + simpleName); @@ -467,10 +464,7 @@ public void performUpdateAuthorisation(String user, EntityBaseBean bean, JsonObj if (updaters.contains(field)) { String qName = Rule.UPDATE_ATTRIBUTE_QUERY; logger.debug("Checking " + qName + " " + user + " " + simpleName + "." + fName); - TypedQuery query = manager.createNamedQuery(qName, String.class) - .setParameter("member", user).setParameter("bean", simpleName) - .setParameter("attribute", fName); - List restrictions = query.getResultList(); + List restrictions = gateKeeperHelper.getRules(qName, user, simpleName, fName); logger.debug("Got " + restrictions.size() + " authz queries for UPDATE by " + user + " to a " + simpleName + "." + fName); boolean ok = false; @@ -569,33 +563,14 @@ public void updateCache() throws JMSException { } public void updatePublicSteps() { - List steps = gateKeeperManager.createNamedQuery(PublicStep.GET_ALL_QUERY, PublicStep.class) - .getResultList(); - publicSteps.clear(); - for (PublicStep step : steps) { - Set fieldNames = publicSteps.get(step.getOrigin()); - if (fieldNames == null) { - fieldNames = new ConcurrentSkipListSet<>(); - publicSteps.put(step.getOrigin(), fieldNames); - } - fieldNames.add(step.getField()); - } + publicSteps = gateKeeperHelper.getPublicSteps(); publicStepsStale = false; - logger.debug("There are " + steps.size() + " publicSteps"); + logger.debug("There are " + publicSteps.size() + " publicSteps: " + publicSteps.toString()); } public void updatePublicTables() { - try { - List tableNames = gateKeeperManager.createNamedQuery(Rule.PUBLIC_QUERY, String.class) - .getResultList(); - publicTables.clear(); - publicTables.addAll(tableNames); - publicTablesStale = false; - logger.debug("There are " + publicTables.size() + " publicTables"); - } catch (Exception e) { - e.printStackTrace(); - logger.error("Unexpected exception", e); - } + publicTables = gateKeeperHelper.getPublicTables(); + publicTablesStale = false; + logger.debug("There are " + publicTables.size() + " publicTables: " + publicTables.toString()); } - } diff --git a/src/main/java/org/icatproject/core/manager/GateKeeperHelper.java b/src/main/java/org/icatproject/core/manager/GateKeeperHelper.java new file mode 100644 index 00000000..3aa5721b --- /dev/null +++ b/src/main/java/org/icatproject/core/manager/GateKeeperHelper.java @@ -0,0 +1,74 @@ +package org.icatproject.core.manager; + +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import jakarta.ejb.Stateless; +import jakarta.ejb.TransactionManagement; +import jakarta.ejb.TransactionManagementType; +import jakarta.persistence.EntityManager; +import jakarta.persistence.PersistenceContext; + +import org.icatproject.core.entity.PublicStep; +import org.icatproject.core.entity.Rule; + +/* + * This class contains methods that provide authorization rules from the + * database to the GateKeeper. They need to be run outside of the transaction + * that is active in GateKeeper, so that they cannot be affected by any changes + * made in that transaction. This is acheived by having them in a separate EJB + * with bean-managed transactions (these never use the transaction of the + * calling bean). + */ +@Stateless +@TransactionManagement(TransactionManagementType.BEAN) +public class GateKeeperHelper { + + @PersistenceContext(unitName = "icat") + private EntityManager gateKeeperManager; + + public List getRules(String ruleQuery, String member, String bean, String attribute) { + return gateKeeperManager + .createNamedQuery(ruleQuery, String.class) + .setParameter("member", member) + .setParameter("bean", bean) + .setParameter("attribute", attribute) + .getResultList(); + } + + public List getRules(String ruleQuery, String member, String bean) { + return gateKeeperManager + .createNamedQuery(ruleQuery, String.class) + .setParameter("member", member) + .setParameter("bean", bean) + .getResultList(); + } + + public Map> getPublicSteps() { + Map> publicSteps = new HashMap<>(); + List steps = gateKeeperManager.createNamedQuery(PublicStep.GET_ALL_QUERY, PublicStep.class).getResultList(); + + for (PublicStep step : steps) { + Set fieldNames = publicSteps.get(step.getOrigin()); + if (fieldNames == null) { + fieldNames = new HashSet<>(); + publicSteps.put(step.getOrigin(), fieldNames); + } + fieldNames.add(step.getField()); + } + + // return unmodifiable copy + publicSteps.replaceAll((k, v) -> Set.copyOf(v)); + return Map.copyOf(publicSteps); + } + + public Set getPublicTables() { + List tableNames = gateKeeperManager.createNamedQuery(Rule.PUBLIC_QUERY, String.class).getResultList(); + + // return unmodifiable copy + return Set.copyOf(tableNames); + } +}