diff --git a/.github/workflows/ci-build.yml b/.github/workflows/ci-build.yml index 084ded90..928aa60e 100644 --- a/.github/workflows/ci-build.yml +++ b/.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 diff --git a/README.md b/README.md index b6d09178..a298cf3a 100644 --- a/README.md +++ b/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 diff --git a/pom.xml b/pom.xml index b6ec5b3a..79a4fd3a 100644 --- a/pom.xml +++ b/pom.xml @@ -106,7 +106,7 @@ org.eclipse.parsson parsson - 1.1.0 + 1.1.3 test diff --git a/src/main/java/org/icatproject/core/manager/EntityInfoHandler.java b/src/main/java/org/icatproject/core/manager/EntityInfoHandler.java index 30ad927d..145bdf0a 100644 --- a/src/main/java/org/icatproject/core/manager/EntityInfoHandler.java +++ b/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> 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 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> 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, 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 getClass(String tableName) throws IcatException { Class entityClass = ENTITY_NAME_MAP.get(tableName); 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); + } +} diff --git a/src/site/xhtml/release-notes.xhtml b/src/site/xhtml/release-notes.xhtml index aed82dcb..613c2d64 100644 --- a/src/site/xhtml/release-notes.xhtml +++ b/src/site/xhtml/release-notes.xhtml @@ -18,6 +18,9 @@
  • Support for synonym injection
  • +

    6.0.1

    +

    Ensures that authorization rules are read in a separate transaction.

    +

    6.0.0

    Upgrade from JavaEE to JakartaEE 10. Requires Java 11+ and an application server that supports JakartaEE 10 such as Payara 6.

    diff --git a/src/test/java/org/icatproject/core/manager/TestEntityInfo.java b/src/test/java/org/icatproject/core/manager/TestEntityInfo.java index 295ae7ee..c44fd686 100644 --- a/src/test/java/org/icatproject/core/manager/TestEntityInfo.java +++ b/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 klass, String... fieldNames) throws Exception { Set results = EntityInfoHandler.getRelInKey(klass); Set rStrings = new HashSet();