Merge branch 'master' into 267_opensearch_support

.github/workflows/ci-build.yml

@@ -11,7 +11,7 @@ jobs:
     runs-on: ubuntu-22.04
     strategy:
       matrix:
-        java_version: [11, 17]
+        java_version: [11, 21]
 
     steps:
       # Setup Java
@@ -25,7 +25,7 @@ jobs:
         run: sudo apt-get update
 
       - name: Cache local Maven repository
-        uses: actions/cache@704facf57e6136b1bc63b828d79edcd491f0ee84 # v3.3.2
+        uses: actions/cache@6849a6489940f00c2f30c0fb92c6274307ccb58a # v4.1.2
         with:
           path: ~/.m2/repository
           key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }}
@@ -34,11 +34,10 @@ jobs:
 
       # ICAT Ansible clone and install dependencies
       - name: Checkout icat-ansible
-        uses: actions/checkout@cd7d8d697e10461458bc61a30d094dc601a8b017 # v4.0.0
+        uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.0.0
         with:
           repository: icatproject-contrib/icat-ansible
           path: icat-ansible
-          ref: payara6
       - name: Install Ansible
         run: pip install -r icat-ansible/requirements.txt
 
@@ -70,7 +69,7 @@ jobs:
           ansible-playbook icat-ansible/icat_server_dev_hosts.yml -i icat-ansible/hosts --vault-password-file icat-ansible/vault_pass.txt -vv
 
       - name: Checkout icat-server
-        uses: actions/checkout@cd7d8d697e10461458bc61a30d094dc601a8b017 # v4.0.0
+        uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.0.0
 
       # Payara must be sourced otherwise the Maven build command fails
       - name: Run Build

README.md

@@ -4,6 +4,6 @@
 
 General installation instructions are at https://icatproject.org/installation/component
 
-Specific installation instructions are at https://repo.icatproject.org/site/icat/server/6.0.0/installation.html
+Specific installation instructions are at https://repo.icatproject.org/site/icat/server/6.0.1/installation.html
 
-All documentation on the icat.server may be found at https://repo.icatproject.org/site/icat/server/6.0.0
+All documentation on the icat.server may be found at https://repo.icatproject.org/site/icat/server/6.0.1

pom.xml

@@ -106,7 +106,7 @@
 		<dependency>
 			<groupId>org.eclipse.parsson</groupId>
 			<artifactId>parsson</artifactId>
-			<version>1.1.0</version>
+			<version>1.1.3</version>
 			<scope>test</scope>
 		</dependency>
 

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

@@ -18,6 +18,7 @@
 import java.util.Map.Entry;
 import java.util.Set;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import jakarta.json.stream.JsonGenerator;
 import jakarta.persistence.CascadeType;
@@ -70,6 +71,7 @@
 import org.icatproject.core.entity.InvestigationUser;
 import org.icatproject.core.entity.Job;
 import org.icatproject.core.entity.Keyword;
+import org.icatproject.core.entity.Parameter;
 import org.icatproject.core.entity.ParameterType;
 import org.icatproject.core.entity.PermissibleStringValue;
 import org.icatproject.core.entity.PublicStep;
@@ -223,6 +225,10 @@ public String toString() {
 			InvestigationInstrument.class, InstrumentScientist.class, DatasetInstrument.class,
 			InvestigationFacilityCycle.class);
 
+	// All entities, plus the abstract classes Parameter and EntityBaseBean
+	private static final List<Class<? extends EntityBaseBean>> EXTENDED_ENTITIES =
+		Stream.concat(ENTITIES.stream(), List.of(Parameter.class, EntityBaseBean.class).stream()).collect(Collectors.toUnmodifiableList());
+
 	// All entity names in export order
 	private static final List<String> EXPORT_ENTITY_NAMES =
 		ENTITIES.stream().map((entity) -> entity.getSimpleName()).collect(Collectors.toUnmodifiableList());
@@ -233,11 +239,11 @@ public String toString() {
 
 	// Map of entity name -> entity class
 	private static final Map<String, Class<? extends EntityBaseBean>> ENTITY_NAME_MAP =
-		ENTITIES.stream().collect(Collectors.toUnmodifiableMap((entity) -> entity.getSimpleName(), (entity) -> entity));
+		EXTENDED_ENTITIES.stream().collect(Collectors.toUnmodifiableMap((entity) -> entity.getSimpleName(), (entity) -> entity));
 
 	// Map of entity class -> PrivateEntityInfo
 	private static final Map<Class<? extends EntityBaseBean>, PrivateEntityInfo> PRIVATE_ENTITY_INFO_MAP =
-		ENTITIES.stream().collect(Collectors.toUnmodifiableMap((entity) -> entity, (entity) -> buildEi(entity)));
+		EXTENDED_ENTITIES.stream().collect(Collectors.toUnmodifiableMap((entity) -> entity, (entity) -> buildEi(entity)));
 
 	public static Class<? extends EntityBaseBean> getClass(String tableName) throws IcatException {
 		Class<? extends EntityBaseBean> entityClass = ENTITY_NAME_MAP.get(tableName);

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<String, Set<String>> publicSteps = new ConcurrentSkipListMap<>();
+	private Map<String, Set<String>> publicSteps;
 
-	private Set<String> publicTables = new ConcurrentSkipListSet<>();
+	private Set<String> publicTables;
 
 	private Set<String> 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<String> fieldNames = publicSteps.get(originBeanName);
+		Set<String> 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<String, Set<String>> getPublicSteps() {
+		if (publicStepsStale) {
+			updatePublicSteps();
+		}
+		return publicSteps;
+	}
+
 	public Set<String> getPublicTables() {
 		if (publicTablesStale) {
 			updatePublicTables();
@@ -191,10 +193,7 @@ private List<String> getRestrictions(String userId, String simpleName, EntityMan
 			return null;
 		}
 
-		TypedQuery<String> query = manager.createNamedQuery(Rule.INCLUDE_QUERY, String.class)
-				.setParameter("member", userId).setParameter("bean", simpleName);
-
-		List<String> restrictions = query.getResultList();
+		List<String> 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<String> query = manager.createNamedQuery(qName, String.class).setParameter("member", user)
-				.setParameter("bean", simpleName);
-		List<String> restrictions = query.getResultList();
+		List<String> 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<String> query = manager.createNamedQuery(qName, String.class)
-							.setParameter("member", user).setParameter("bean", simpleName)
-							.setParameter("attribute", fName);
-					List<String> restrictions = query.getResultList();
+					List<String> 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<PublicStep> steps = gateKeeperManager.createNamedQuery(PublicStep.GET_ALL_QUERY, PublicStep.class)
-				.getResultList();
-		publicSteps.clear();
-		for (PublicStep step : steps) {
-			Set<String> 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<String> 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());
 	}
-
 }

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<String> 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<String> getRules(String ruleQuery, String member, String bean) {
+		return gateKeeperManager
+			.createNamedQuery(ruleQuery, String.class)
+			.setParameter("member", member)
+			.setParameter("bean", bean)
+			.getResultList();
+	}
+
+	public Map<String, Set<String>> getPublicSteps() {
+		Map<String, Set<String>> publicSteps = new HashMap<>();
+		List<PublicStep> steps = gateKeeperManager.createNamedQuery(PublicStep.GET_ALL_QUERY, PublicStep.class).getResultList();
+
+		for (PublicStep step : steps) {
+			Set<String> 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<String> getPublicTables() {
+		List<String> tableNames = gateKeeperManager.createNamedQuery(Rule.PUBLIC_QUERY, String.class).getResultList();
+
+		// return unmodifiable copy
+		return Set.copyOf(tableNames);
+	}
+}

src/site/xhtml/release-notes.xhtml

@@ -18,6 +18,9 @@
 		<li>Support for synonym injection</li>
 	</ul>
 
+	<h2>6.0.1</h2>
+	<p>Ensures that authorization rules are read in a separate transaction.</p>
+
 	<h2>6.0.0</h2>
 	<p>Upgrade from JavaEE to JakartaEE 10. Requires Java 11+ and an application server that supports JakartaEE 10 such as Payara 6.</p>
 

src/test/java/org/icatproject/core/manager/TestEntityInfo.java

@@ -2,6 +2,7 @@
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
 import java.lang.reflect.Field;
@@ -433,6 +434,14 @@ public void relInKey() throws Exception {
 		testRelInkey(DataCollectionDatafile.class, "dataCollection", "datafile");
 	}
 
+	@Test
+	public void testAbstractEntities() throws Exception {
+		assertNotNull(EntityInfoHandler.getClass("Parameter"));
+		assertNotNull(EntityInfoHandler.getEntityInfo("Parameter"));
+		assertNotNull(EntityInfoHandler.getClass("EntityBaseBean"));
+		assertNotNull(EntityInfoHandler.getEntityInfo("EntityBaseBean"));
+	}
+
 	private void testRelInkey(Class<? extends EntityBaseBean> klass, String... fieldNames) throws Exception {
 		Set<Field> results = EntityInfoHandler.getRelInKey(klass);
 		Set<String> rStrings = new HashSet<String>();